From c555bb519c122a4ecda9fd506d9fe8fde0c7ddcb Mon Sep 17 00:00:00 2001 From: Tom Udding Date: Sun, 30 Aug 2026 14:40:01 +0200 Subject: [PATCH 1/5] feat(user): let a member change their own details Closes GH-118 --- config/packages/messenger.yaml | 4 + config/reference.php | 4 +- migrations/database/Version20260923140501.php | 46 ++ phpstan-baseline.neon | 12 - src/Controller/User/EmailChangeController.php | 196 ++++++++ .../User/MemberDetailsController.php | 414 +++++++++++++++++ .../Mailing/MailingListFixture.php | 9 +- .../Mailing/MailingListMembershipFixture.php | 31 ++ src/Entity/Database/ActionLink.php | 1 + src/Entity/Database/AuditAddressChange.php | 69 +++ src/Entity/Database/AuditEmailChange.php | 62 +++ src/Entity/Database/AuditEntry.php | 2 + src/Entity/Database/EmailChangeLink.php | 68 +++ .../Enums/MailingListMemberOrigin.php | 2 + .../Database/Enums/MemberDetailAction.php | 37 ++ src/Entity/Database/MailingList.php | 12 + .../Database/MemberEmailChangeListener.php | 187 ++++++++ src/Form/Database/MailingListLabel.php | 34 +- src/Form/Database/MailingListType.php | 13 + src/Form/Database/MemberListsType.php | 88 +++- src/Form/User/MemberEmailChangeType.php | 65 +++ .../Database/EmailChangeConfirmationEmail.php | 30 ++ .../Database/EmailChangedNoticeEmail.php | 33 ++ .../EmailChangeConfirmationEmailHandler.php | 57 +++ .../EmailChangedNoticeEmailHandler.php | 45 ++ .../Database/ActionLinkRepository.php | 26 ++ .../Database/AuditAddressChangeRepository.php | 23 + .../Database/AuditEmailChangeRepository.php | 23 + .../Database/EmailChangeLinkRepository.php | 40 ++ .../Database/MailingListRepository.php | 11 + src/Service/Database/ActionLinkService.php | 23 + src/Service/Database/Member.php | 144 +++++- .../email/email-change-confirm.html.twig | 24 + .../database/email/email-changed.html.twig | 22 + .../database/mailing/list/form.html.twig | 1 + templates/partials/member-sidebar.html.twig | 1 + templates/user/email-change.html.twig | 40 ++ .../settings/details-address-remove.html.twig | 35 ++ .../user/settings/details-address.html.twig | 41 ++ templates/user/settings/details.html.twig | 142 ++++++ tests/Entity/Database/EmailChangeLinkTest.php | 71 +++ .../User/EmailChangeControllerTest.php | 255 +++++++++++ .../User/MemberDetailsControllerTest.php | 421 ++++++++++++++++++ .../MemberEmailChangeListenerTest.php | 152 +++++++ .../Database/EmailChangeEmailsTest.php | 147 ++++++ .../Database/ActionLinkServiceTest.php | 105 +++++ translations/AutocompleteBundle.en.xlf | 26 ++ translations/AutocompleteBundle.nl.xlf | 26 ++ translations/messages.en.xlf | 112 +++++ translations/messages.nl.xlf | 112 +++++ 50 files changed, 3487 insertions(+), 57 deletions(-) create mode 100644 migrations/database/Version20260923140501.php create mode 100644 src/Controller/User/EmailChangeController.php create mode 100644 src/Controller/User/MemberDetailsController.php create mode 100644 src/Entity/Database/AuditAddressChange.php create mode 100644 src/Entity/Database/AuditEmailChange.php create mode 100644 src/Entity/Database/EmailChangeLink.php create mode 100644 src/Entity/Database/Enums/MemberDetailAction.php create mode 100644 src/EventListener/Database/MemberEmailChangeListener.php create mode 100644 src/Form/User/MemberEmailChangeType.php create mode 100644 src/Message/Database/EmailChangeConfirmationEmail.php create mode 100644 src/Message/Database/EmailChangedNoticeEmail.php create mode 100644 src/MessageHandler/Database/EmailChangeConfirmationEmailHandler.php create mode 100644 src/MessageHandler/Database/EmailChangedNoticeEmailHandler.php create mode 100644 src/Repository/Database/AuditAddressChangeRepository.php create mode 100644 src/Repository/Database/AuditEmailChangeRepository.php create mode 100644 src/Repository/Database/EmailChangeLinkRepository.php create mode 100644 templates/database/email/email-change-confirm.html.twig create mode 100644 templates/database/email/email-changed.html.twig create mode 100644 templates/user/email-change.html.twig create mode 100644 templates/user/settings/details-address-remove.html.twig create mode 100644 templates/user/settings/details-address.html.twig create mode 100644 templates/user/settings/details.html.twig create mode 100644 tests/Entity/Database/EmailChangeLinkTest.php create mode 100644 tests/Integration/Controller/User/EmailChangeControllerTest.php create mode 100644 tests/Integration/Controller/User/MemberDetailsControllerTest.php create mode 100644 tests/Integration/EventListener/Database/MemberEmailChangeListenerTest.php create mode 100644 tests/Integration/MessageHandler/Database/EmailChangeEmailsTest.php create mode 100644 tests/Service/Database/ActionLinkServiceTest.php create mode 100644 translations/AutocompleteBundle.en.xlf create mode 100644 translations/AutocompleteBundle.nl.xlf diff --git a/config/packages/messenger.yaml b/config/packages/messenger.yaml index 16bac33d7..6140825fd 100644 --- a/config/packages/messenger.yaml +++ b/config/packages/messenger.yaml @@ -145,6 +145,10 @@ framework: # which Stripe repeats the same event) or a page that hangs on the MTA. 'App\Message\Database\RegistrationUpdateEmail': high_priority 'App\Message\Database\RefundProblemEmail': high_priority + # A member waiting on a page for a confirmation link, and the address that has just been replaced hearing + # that it was, are both worth little late. + 'App\Message\Database\EmailChangeConfirmationEmail': high_priority + 'App\Message\Database\EmailChangedNoticeEmail': high_priority 'App\Message\User\ExportUserDataMessage': bulk 'App\Message\User\RevokeSessionsRealtimeMessage': high_priority # A security notice is worth little late, and it must not delay whatever raised it. diff --git a/config/reference.php b/config/reference.php index d42deda65..0004a15cb 100644 --- a/config/reference.php +++ b/config/reference.php @@ -1846,8 +1846,8 @@ * } * @psalm-type MercureConfig = array{ * hubs?: arrayaddSql('ALTER TABLE ActionLink ADD newEmail VARCHAR(255) DEFAULT NULL'); + $this->addSql('ALTER TABLE ActionLink ADD previousEmail VARCHAR(255) DEFAULT NULL'); + $this->addSql('ALTER TABLE ActionLink ADD requestedOn TIMESTAMP(0) WITHOUT TIME ZONE DEFAULT NULL'); + $this->addSql('ALTER TABLE AuditEntry ADD addressType VARCHAR(255) DEFAULT NULL'); + $this->addSql('ALTER TABLE AuditEntry ADD detailAction VARCHAR(255) DEFAULT NULL'); + $this->addSql('ALTER TABLE AuditEntry ADD oldEmail VARCHAR(255) DEFAULT NULL'); + $this->addSql('ALTER TABLE AuditEntry ADD newEmail VARCHAR(255) DEFAULT NULL'); + $this->addSql('ALTER TABLE MailingList ADD selfService BOOLEAN DEFAULT false NOT NULL'); + } + + public function down(Schema $schema): void + { + $this->addSql('ALTER TABLE ActionLink DROP newEmail'); + $this->addSql('ALTER TABLE ActionLink DROP previousEmail'); + $this->addSql('ALTER TABLE ActionLink DROP requestedOn'); + $this->addSql('ALTER TABLE AuditEntry DROP addressType'); + $this->addSql('ALTER TABLE AuditEntry DROP detailAction'); + $this->addSql('ALTER TABLE AuditEntry DROP oldEmail'); + $this->addSql('ALTER TABLE AuditEntry DROP newEmail'); + $this->addSql('ALTER TABLE MailingList DROP selfService'); + } +} diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 2de6d9494..63b24b4f8 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1164,24 +1164,12 @@ parameters: count: 1 path: src/Service/Database/MailmanService.php - - - message: '#^Cannot access property \$email on App\\Entity\\Database\\MailingListMember\|null\.$#' - identifier: property.nonObject - count: 1 - path: src/Service/Database/Member.php - - message: '#^Cannot access property \$endDate on App\\Entity\\Database\\Membership\|null\.$#' identifier: property.nonObject count: 3 path: src/Service/Database/Member.php - - - message: '#^Cannot access property \$toBeDeleted on App\\Entity\\Database\\MailingListMember\|null\.$#' - identifier: property.nonObject - count: 1 - path: src/Service/Database/Member.php - - message: '#^Cannot access property \$type on App\\Entity\\Database\\Membership\|null\.$#' identifier: property.nonObject diff --git a/src/Controller/User/EmailChangeController.php b/src/Controller/User/EmailChangeController.php new file mode 100644 index 000000000..7643d7813 --- /dev/null +++ b/src/Controller/User/EmailChangeController.php @@ -0,0 +1,196 @@ + '[0-9a-f]{32}\.[0-9a-f]{96}'], + methods: ['GET'], + )] + public function claim(string $token): Response + { + $link = $this->actionLinkService->resolveEmailChange($token); + + if (null === $link) { + return $this->withNoLeakHeaders($this->render( + 'user/email-change.html.twig', + ['link' => null], + )); + } + + return $this->withNoLeakHeaders($this->redirectToRoute( + 'user_email_change_confirm', + ['th' => $this->actionLinkService->claim($link)], + )); + } + + #[Route( + path: '/user/email-change', + name: 'user_email_change_confirm', + methods: ['GET'], + )] + public function confirm(Request $request): Response + { + $tempHash = (string) $request->query->get( + 'th', + '', + ); + + if ('' !== $tempHash) { + $claimed = $this->actionLinkService->findByTempHash($tempHash); + + if ($claimed instanceof EmailChangeLink) { + // Single-use: spent as the page is handed over rather than once it is acted on. + $this->actionLinkService->consumeTempHash($claimed); + + $request->getSession()->set( + self::SESSION_KEY, + $claimed->id, + ); + } + + return $this->withNoLeakHeaders($this->redirectToRoute('user_email_change_confirm')); + } + + $this->denyAccessUnlessGranted( + UserRoles::User->value, + message: 'You are not allowed to change these details.', + ); + + $user = $this->getUser(); + + return $this->withNoLeakHeaders($this->render( + 'user/email-change.html.twig', + [ + 'link' => $user instanceof User + ? $this->pendingLink( + $request, + $user, + ) + : null, + ], + )); + } + + #[IsGranted( + attribute: UserRoles::User->value, + message: 'You are not allowed to change these details.', + )] + #[IsCsrfTokenValid( + id: 'email_change', + tokenKey: '_csrf_token', + )] + #[Route( + path: '/user/email-change', + name: 'user_email_change_apply', + methods: ['POST'], + )] + public function apply( + Request $request, + #[CurrentUser] + User $user, + ): Response { + $link = $this->pendingLink( + $request, + $user, + ); + + if (null === $link) { + return $this->withNoLeakHeaders($this->render( + 'user/email-change.html.twig', + ['link' => null], + )); + } + + $previousEmail = $link->previousEmail; + $member = $this->memberService->confirmEmailChange($link); + + $request->getSession()->remove(self::SESSION_KEY); + + // The replaced address is the one that still reaches a member whose account was taken. + if (null !== $previousEmail) { + $this->bus->dispatch(new EmailChangedNoticeEmail( + $member->lidnr, + $previousEmail, + $link->newEmail, + )); + } + + $this->addFlash( + AlertTypes::Success->value, + $this->translator->trans('Your e-mail address has been changed. You now sign in with it as well.'), + ); + + return $this->redirectToRoute('user_settings_details_index'); + } + + private function pendingLink( + Request $request, + User $user, + ): ?EmailChangeLink { + $linkId = $request->getSession()->get(self::SESSION_KEY); + + if (!is_int($linkId)) { + return null; + } + + $link = $this->emailChangeLinkRepository->find($linkId); + + if ( + null === $link + || $link->used + || $link->linkExpired() + || $link->member->lidnr !== $user->member->lidnr + ) { + $request->getSession()->remove(self::SESSION_KEY); + + return null; + } + + return $link; + } +} diff --git a/src/Controller/User/MemberDetailsController.php b/src/Controller/User/MemberDetailsController.php new file mode 100644 index 000000000..1e3f3e76d --- /dev/null +++ b/src/Controller/User/MemberDetailsController.php @@ -0,0 +1,414 @@ +value, + message: 'You are not allowed to change these details.', +)] +#[Route( + path: '/user/settings/details', + name: 'user_settings_details_', +)] +class MemberDetailsController extends AbstractController +{ + public function __construct( + private readonly TranslatorInterface $translator, + private readonly MemberService $memberService, + private readonly MemberRepository $memberRepository, + private readonly MailingListRepository $mailingListRepository, + private readonly MessageBusInterface $bus, + ) { + } + + #[Route( + path: '', + name: 'index', + methods: [ + 'GET', + 'POST', + ], + )] + public function index( + Request $request, + #[CurrentUser] + User $user, + ): Response { + $member = $this->member($user); + $syncLocked = $this->memberService->isMailingListSyncLocked(); + $selfServiceLists = $this->mailingListRepository->findAllSelfService(); + + $emailForm = $this->createForm(MemberEmailChangeType::class)->handleRequest($request); + + if ($emailForm->isSubmitted()) { + $response = $this->handleEmailChange( + $emailForm, + $member, + ); + + if (null !== $response) { + return $response; + } + } + + $listsForm = $this->createForm( + MemberListsType::class, + null, + [ + 'member' => $member, + 'lists' => $selfServiceLists, + // What a list is for is worth reading before ticking it, and the panel already says these are the + // mailing lists. + 'describe' => true, + ], + )->handleRequest($request); + + if ( + $listsForm->isSubmitted() + && $listsForm->isValid() + ) { + $response = $this->handleSubscriptions( + $listsForm, + $member, + $selfServiceLists, + ); + + if (null !== $response) { + return $response; + } + } + + return $this->render( + 'user/settings/details.html.twig', + [ + 'member' => $member, + 'emailForm' => $emailForm, + 'listsForm' => $listsForm, + // While a synchronisation is running the pending states on the subscriptions are being turned into + // real ones, so what the page would submit could not be saved anyway. + 'syncLocked' => $syncLocked, + 'selfServiceLists' => $selfServiceLists, + 'addresses' => $this->addresses($member), + ], + ); + } + + /** + * The three addresses a member can have on file, in the order the page shows them, each with what is on file for + * it or null. The register holds them as a collection of whichever ones exist, and a page that has to offer + * adding one needs to know which are missing. + * + * @return list + */ + private function addresses(MemberModel $member): array + { + $addresses = []; + + foreach (AddressTypes::cases() as $type) { + $addresses[] = [ + 'type' => $type, + 'address' => $this->memberService->getAddress( + $member, + $type, + ), + ]; + } + + return $addresses; + } + + /** + * Add or correct one of the three addresses. Which one is being written follows from the address of the page, + * never from what is submitted. + */ + #[Route( + path: '/address/{type}', + name: 'address', + requirements: ['type' => 'home|student|mail'], + methods: [ + 'GET', + 'POST', + ], + )] + public function address( + Request $request, + AddressTypes $type, + #[CurrentUser] + User $user, + ): Response { + $member = $this->member($user); + $existing = $this->memberService->getAddress( + $member, + $type, + ); + $address = $existing ?? $this->memberService->getAddress( + $member, + $type, + true, + ); + + $form = $this->createForm( + AddressEditType::class, + $address, + )->handleRequest($request); + + if ( + $form->isSubmitted() + && $form->isValid() + ) { + if (null === $existing) { + $this->memberService->addAddress($form); + } else { + $this->memberService->editAddress($form); + } + + $this->addFlash( + AlertTypes::Success->value, + $this->translator->trans('Your address has been saved.'), + ); + + return $this->redirectToRoute('user_settings_details_index'); + } + + return $this->render( + 'user/settings/details-address.html.twig', + [ + 'add' => null === $existing, + 'addressType' => $type, + 'form' => $form, + ], + ); + } + + #[Route( + path: '/address/{type}/remove', + name: 'address_remove', + requirements: ['type' => 'home|student|mail'], + methods: [ + 'GET', + 'POST', + ], + )] + public function removeAddress( + Request $request, + AddressTypes $type, + #[CurrentUser] + User $user, + ): Response { + $member = $this->member($user); + + if ( + null === $this->memberService->getAddress( + $member, + $type, + ) + ) { + throw $this->createNotFoundException(); + } + + $form = $this->createForm(DeleteAddressType::class)->handleRequest($request); + + // `isValid()` before the button: a clicked button says nothing about the token. + if ( + $form->isSubmitted() + && $form->isValid() + ) { + if ( + SubmitButtons::clicked( + $form, + 'submit_yes', + ) + ) { + $this->memberService->removeAddress( + $member, + $type, + $form, + ); + + $this->addFlash( + AlertTypes::Success->value, + $this->translator->trans('Your address has been removed.'), + ); + } + + return $this->redirectToRoute('user_settings_details_index'); + } + + return $this->render( + 'user/settings/details-address-remove.html.twig', + [ + 'addressType' => $type, + 'form' => $form, + ], + ); + } + + /** + * The member the account belongs to, as the register holds them. + * + * The account carries the projection's member, which is what the website reads; what this page writes is the + * ledger, so it asks the register for the same member. An account without one cannot exist -- it is removed with + * the member -- so a member that is not there is a broken installation rather than a page to render. + */ + private function member(User $user): MemberModel + { + $member = $this->memberRepository->find($user->member->lidnr); + + if (null === $member) { + throw $this->createNotFoundException(); + } + + return $member; + } + + /** + * Answers a response when the page is done with, and null when the form should be rendered again with what is + * wrong with it. + * + * @param FormInterface|null> $form + */ + private function handleEmailChange( + FormInterface $form, + MemberModel $member, + ): ?Response { + if (!$form->isValid()) { + return null; + } + + $email = (string) $form->get('email')->getData(); + + if ($email === $member->email) { + $form->get('email')->addError(new FormError( + $this->translator->trans('This is already your e-mail address.'), + )); + + return null; + } + + // Two records answering to one address cannot both be reached, so an address that is taken is refused. This + // says only that it is taken, not by whom. + if ( + $this->memberService->emailBelongsToSomeoneElse( + $email, + $member, + ) + ) { + $form->get('email')->addError(new FormError( + $this->translator->trans('There already is a member with this e-mail address.'), + )); + + return null; + } + + $link = $this->memberService->requestEmailChange( + $member, + $email, + ); + + // The token exists for this request only: the register keeps a hash of it, so it goes into the message now or + // it is gone. + $this->bus->dispatch(new EmailChangeConfirmationEmail( + $member->lidnr, + $email, + (string) $link->plainToken, + )); + + $this->addFlash( + AlertTypes::Info->value, + $this->translator->trans( + // phpcs:ignore -- user-visible strings should not be split + 'We have sent a message to your new e-mail address. Your address changes once you have followed the link in it.', + ), + ); + + return $this->redirectToRoute('user_settings_details_index'); + } + + /** + * @param FormInterface|null> $form + * @param array $selfServiceLists + */ + private function handleSubscriptions( + FormInterface $form, + MemberModel $member, + array $selfServiceLists, + ): ?Response { + $data = $form->getData(); + /** @var string[] $selected */ + $selected = $data['lists'] ?? []; + + $saved = $this->memberService->updateSubscriptions( + $member, + $selected, + array_map( + static fn (MailingListModel $list): string => $list->name, + $selfServiceLists, + ), + MailingListMemberOrigin::SelfService, + ); + + // A sync that started while the page was open takes precedence, and the subscriptions are left alone. + if (null === $saved) { + $this->addFlash( + AlertTypes::Danger->value, + $this->translator->trans( + // phpcs:ignore -- user-visible strings should not be split + 'Your subscriptions could not be saved because they are being synchronised. Please try again later.', + ), + ); + + return null; + } + + $this->addFlash( + AlertTypes::Success->value, + $this->translator->trans('Your subscriptions have been saved.'), + ); + + return $this->redirectToRoute('user_settings_details_index'); + } +} diff --git a/src/DataFixtures/Mailing/MailingListFixture.php b/src/DataFixtures/Mailing/MailingListFixture.php index c4ee3a43c..e6bf1b912 100644 --- a/src/DataFixtures/Mailing/MailingListFixture.php +++ b/src/DataFixtures/Mailing/MailingListFixture.php @@ -35,8 +35,9 @@ class MailingListFixture extends Fixture implements FixtureGroupInterface public const string REF_LIST_ACTIVITIES = 'list-activities'; /** - * Name, descriptions, whether the registration form offers it, whether it is ticked, and which server it is on: - * the Listmonk id the seed gives it, or null where Mailman has it instead. + * Name, descriptions, whether the registration form offers it, whether it is ticked, whether a member may put + * themselves on it afterwards, and which server it is on: the Listmonk id the seed gives it, or null where + * Mailman has it instead. */ private const array LISTS = [ [ @@ -46,6 +47,7 @@ class MailingListFixture extends Fixture implements FixtureGroupInterface // Not offered as a choice: every member is on it, which is what the registration form says. 'on_form' => false, 'default' => true, + 'self_service' => false, 'listmonk' => null, 'reference' => self::REF_LIST_ANNOUNCEMENTS, ], @@ -55,6 +57,7 @@ class MailingListFixture extends Fixture implements FixtureGroupInterface 'en' => 'Announcements of activities.', 'on_form' => true, 'default' => true, + 'self_service' => true, 'listmonk' => 2, 'reference' => self::REF_LIST_ACTIVITIES, ], @@ -64,6 +67,7 @@ class MailingListFixture extends Fixture implements FixtureGroupInterface 'en' => 'Vacancies and career opportunities from our partners.', 'on_form' => true, 'default' => false, + 'self_service' => true, 'listmonk' => 3, 'reference' => null, ], @@ -89,6 +93,7 @@ public function load(ObjectManager $manager): void $list->setEnDescription($definition['en']); $list->onForm = $definition['on_form']; $list->defaultSub = $definition['default']; + $list->selfService = $definition['self_service']; if (null === $definition['listmonk']) { $mailman = new MailmanMailingList(); diff --git a/src/DataFixtures/Mailing/MailingListMembershipFixture.php b/src/DataFixtures/Mailing/MailingListMembershipFixture.php index e415e090c..3643b2d9c 100644 --- a/src/DataFixtures/Mailing/MailingListMembershipFixture.php +++ b/src/DataFixtures/Mailing/MailingListMembershipFixture.php @@ -5,6 +5,7 @@ namespace App\DataFixtures\Mailing; use App\DataFixtures\Member\MemberFixture; +use App\DataFixtures\Member\MemberPopulationFixture; use App\Entity\Database\MailingList; use App\Entity\Database\MailingListMember; use App\Entity\Database\Member as MemberModel; @@ -16,6 +17,8 @@ use LogicException; use Override; +use function sprintf; + /** * Who is on which list. * @@ -76,6 +79,33 @@ public function load(ObjectManager $manager): void $manager->persist($this->subscribe($activities, $student)); + // 8000 gets one list a member manages and one they do not, so the settings page shows both cases. + $signIn = $this->getReference( + sprintf( + 'ledger-member-%d', + MemberPopulationFixture::ADMIN, + ), + MemberModel::class, + ); + + $carriedAcross = $this->subscribe( + $activities, + $signIn, + ); + $carriedAcross->toBeCreated = false; + $carriedAcross->setLastSyncOn(); + $carriedAcross->lastSyncSuccess = true; + $manager->persist($carriedAcross); + + $announced = $this->subscribe( + $announcements, + $signIn, + ); + $announced->toBeCreated = false; + $announced->setLastSyncOn(); + $announced->lastSyncSuccess = true; + $manager->persist($announced); + $manager->flush(); } @@ -100,6 +130,7 @@ public function getDependencies(): array return [ MailingListFixture::class, MemberFixture::class, + MemberPopulationFixture::class, ]; } diff --git a/src/Entity/Database/ActionLink.php b/src/Entity/Database/ActionLink.php index 39c13240e..41781e9d1 100644 --- a/src/Entity/Database/ActionLink.php +++ b/src/Entity/Database/ActionLink.php @@ -35,6 +35,7 @@ value: [ 'payment' => PaymentLink::class, 'renewal' => RenewalLink::class, + 'email_change' => EmailChangeLink::class, ], )] #[Index( diff --git a/src/Entity/Database/AuditAddressChange.php b/src/Entity/Database/AuditAddressChange.php new file mode 100644 index 000000000..ca20061d7 --- /dev/null +++ b/src/Entity/Database/AuditAddressChange.php @@ -0,0 +1,69 @@ +%s address of %s was %s'; + + #[Column( + type: 'string', + enumType: AddressTypes::class, + )] + public AddressTypes $addressType; + + /** + * Named apart from the `action` of a mailing list subscription, which is a column of the same table. + */ + #[Column( + type: 'string', + enumType: MemberDetailAction::class, + )] + public MemberDetailAction $detailAction; + + public static function create( + Member $member, + AddressTypes $addressType, + MemberDetailAction $action, + ?Member $user = null, + ): self { + $audit = new self(); + $audit->setMember($member); + $audit->addressType = $addressType; + $audit->detailAction = $action; + $audit->user = $user; + + return $audit; + } + + #[Override] + protected function getStringBodyFormatted(): string + { + return self::BODY_FORMAT; + } + + /** + * @return array + */ + #[Override] + protected function getStringArguments(): array + { + return [ + $this->addressType->value, + $this->member?->getFullName() ?? '-', + $this->detailAction->value, + ]; + } +} diff --git a/src/Entity/Database/AuditEmailChange.php b/src/Entity/Database/AuditEmailChange.php new file mode 100644 index 000000000..fb3897195 --- /dev/null +++ b/src/Entity/Database/AuditEmailChange.php @@ -0,0 +1,62 @@ +Changed e-mail address of %s from ' + . '%s to %s'; + + #[Column( + type: 'string', + nullable: true, + )] + public ?string $oldEmail = null; + + #[Column(type: 'string')] + public string $newEmail; + + public static function create( + Member $member, + ?string $oldEmail, + string $newEmail, + ?Member $user = null, + ): self { + $audit = new self(); + $audit->setMember($member); + $audit->oldEmail = $oldEmail; + $audit->newEmail = $newEmail; + $audit->user = $user; + + return $audit; + } + + #[Override] + protected function getStringBodyFormatted(): string + { + return self::BODY_FORMAT; + } + + /** + * @return array + */ + #[Override] + protected function getStringArguments(): array + { + return [ + $this->member?->getFullName() ?? '-', + $this->oldEmail ?? '-', + $this->newEmail, + ]; + } +} diff --git a/src/Entity/Database/AuditEntry.php b/src/Entity/Database/AuditEntry.php index e46d291a0..5d932f7dc 100644 --- a/src/Entity/Database/AuditEntry.php +++ b/src/Entity/Database/AuditEntry.php @@ -34,6 +34,8 @@ )] #[DiscriminatorMap( value: [ + 'address_change' => AuditAddressChange::class, + 'email_change' => AuditEmailChange::class, 'mailing_list_membership' => AuditMailingListMembership::class, 'note' => AuditNote::class, 'renewal' => AuditRenewal::class, diff --git a/src/Entity/Database/EmailChangeLink.php b/src/Entity/Database/EmailChangeLink.php new file mode 100644 index 000000000..527148a18 --- /dev/null +++ b/src/Entity/Database/EmailChangeLink.php @@ -0,0 +1,68 @@ +member = $member; + $this->newEmail = $newEmail; + $this->previousEmail = $member->email; + $this->requestedOn = new DateTimeImmutable(); + } + + public function getExpiresAt(): DateTimeImmutable + { + return $this->requestedOn->add(new DateInterval(self::LIFETIME)); + } + + #[Override] + public function linkExpired(): bool + { + return $this->getExpiresAt() <= new DateTimeImmutable(); + } +} diff --git a/src/Entity/Database/Enums/MailingListMemberOrigin.php b/src/Entity/Database/Enums/MailingListMemberOrigin.php index d25482aaf..14b39a12b 100644 --- a/src/Entity/Database/Enums/MailingListMemberOrigin.php +++ b/src/Entity/Database/Enums/MailingListMemberOrigin.php @@ -12,6 +12,7 @@ enum MailingListMemberOrigin: string implements TranslatableInterface { case Manual = 'manual'; + case SelfService = 'self_service'; case SyncMailman = 'sync_mailman'; case SyncListmonk = 'sync_listmonk'; @@ -23,6 +24,7 @@ public function getName(): TranslatableMessage { return match ($this) { self::Manual => new TranslatableMessage('manual'), + self::SelfService => new TranslatableMessage('by the member'), self::SyncMailman => new TranslatableMessage('mailman sync'), self::SyncListmonk => new TranslatableMessage('listmonk sync'), }; diff --git a/src/Entity/Database/Enums/MemberDetailAction.php b/src/Entity/Database/Enums/MemberDetailAction.php new file mode 100644 index 000000000..5868d03e0 --- /dev/null +++ b/src/Entity/Database/Enums/MemberDetailAction.php @@ -0,0 +1,37 @@ + new TranslatableMessage('Added'), + self::Changed => new TranslatableMessage('Changed'), + self::Removed => new TranslatableMessage('Removed'), + }; + } + + #[Override] + public function trans( + TranslatorInterface $translator, + ?string $locale = null, + ): string { + return $this->getName()->trans( + $translator, + $locale, + ); + } +} diff --git a/src/Entity/Database/MailingList.php b/src/Entity/Database/MailingList.php index d6b0cc0d4..f4877ae71 100644 --- a/src/Entity/Database/MailingList.php +++ b/src/Entity/Database/MailingList.php @@ -59,6 +59,16 @@ class MailingList #[Column(type: Types::BOOLEAN)] public bool $defaultSub; + /** + * Whether a member may manage their own subscription. Separate from being on the sign-up form: a list for one + * year is offered when somebody joins, but is not one they may put themselves on later. + */ + #[Column( + type: 'boolean', + options: ['default' => false], + )] + public bool $selfService = false; + /** * The corresponding mailman mailing list */ @@ -188,6 +198,7 @@ public function getAuditEntries(): Collection * en_description: string, * defaultSub: bool, * onForm: bool, + * selfService: bool, * mailmanList: ?string, * listmonkList: ?int, * } @@ -200,6 +211,7 @@ public function toArray(): array 'en_description' => $this->getEnDescription(), 'defaultSub' => $this->defaultSub, 'onForm' => $this->onForm, + 'selfService' => $this->selfService, 'mailmanList' => $this->mailmanList?->mailmanId, 'listmonkList' => $this->listmonkList?->listmonkId, ]; diff --git a/src/EventListener/Database/MemberEmailChangeListener.php b/src/EventListener/Database/MemberEmailChangeListener.php new file mode 100644 index 000000000..64c016c13 --- /dev/null +++ b/src/EventListener/Database/MemberEmailChangeListener.php @@ -0,0 +1,187 @@ +getObjectManager(); + $unitOfWork = $entityManager->getUnitOfWork(); + $metadata = $entityManager->getClassMetadata(MailingListMember::class); + + foreach ($unitOfWork->getScheduledEntityUpdates() as $entity) { + if (!$entity instanceof Member) { + continue; + } + + $changeSet = $unitOfWork->getEntityChangeSet($entity); + + if (!isset($changeSet['email'])) { + continue; + } + + $oldEmail = $changeSet['email'][0] ?? null; + $newEmail = $changeSet['email'][1] ?? null; + + // A member whose address is taken away is being cleared rather than reached somewhere else. + if ( + !is_string($newEmail) + || $oldEmail === $newEmail + ) { + continue; + } + + if (!is_string($oldEmail)) { + // Nothing to carry over, but still something to record. + $this->audit( + $entity, + null, + $newEmail, + $entityManager, + $unitOfWork, + ); + + continue; + } + + $this->repoint( + $entity, + $oldEmail, + $newEmail, + $entityManager, + $unitOfWork, + $metadata, + ); + + $this->audit( + $entity, + $oldEmail, + $newEmail, + $entityManager, + $unitOfWork, + ); + } + } + + /** + * @param ClassMetadata $metadata + */ + private function repoint( + Member $member, + string $oldEmail, + string $newEmail, + EntityManagerInterface $entityManager, + UnitOfWork $unitOfWork, + ClassMetadata $metadata, + ): void { + // Read once: the loop adds to this collection. + $subscriptions = $member->getMailingListMemberships()->toArray(); + + /** @var array $underNewEmail */ + $underNewEmail = []; + foreach ($subscriptions as $subscription) { + if ($newEmail !== $subscription->email) { + continue; + } + + $underNewEmail[$subscription->mailingList->name] = $subscription; + } + + foreach ($subscriptions as $subscription) { + if ( + $oldEmail !== $subscription->email + || $subscription->toBeDeleted + ) { + continue; + } + + $subscription->toBeDeleted = true; + $unitOfWork->recomputeSingleEntityChangeSet( + $metadata, + $subscription, + ); + + $list = $subscription->mailingList; + $existing = $underNewEmail[$list->name] ?? null; + + // The pair (list, address) identifies a row, so one held before is revived rather than written again. + if (null !== $existing) { + $existing->toBeDeleted = false; + $existing->toBeCreated = true; + $unitOfWork->recomputeSingleEntityChangeSet( + $metadata, + $existing, + ); + + continue; + } + + $carried = new MailingListMember(); + $carried->mailingList = $list; + // Sets the address from the member, which by now is the new one. + $carried->setMember($member); + + $entityManager->persist($carried); + $unitOfWork->computeChangeSet( + $metadata, + $carried, + ); + } + } + + private function audit( + Member $member, + ?string $oldEmail, + string $newEmail, + EntityManagerInterface $entityManager, + UnitOfWork $unitOfWork, + ): void { + $user = $this->security->getUser(); + $audit = AuditEmailChange::create( + $member, + $oldEmail, + $newEmail, + $user instanceof User + ? $this->memberRepository->find($user->member->lidnr) + : null, + ); + + $entityManager->persist($audit); + $unitOfWork->computeChangeSet( + $entityManager->getClassMetadata(AuditEmailChange::class), + $audit, + ); + } +} diff --git a/src/Form/Database/MailingListLabel.php b/src/Form/Database/MailingListLabel.php index 7bc2d06a9..857f77277 100644 --- a/src/Form/Database/MailingListLabel.php +++ b/src/Form/Database/MailingListLabel.php @@ -16,18 +16,23 @@ use const ENT_SUBSTITUTE; /** - * Label of a mailing list checkbox on the registration form. + * Label of a mailing list checkbox on the registration form and on a member's own settings page. * * The name goes on its own line above a muted description, so the two read as a heading and its explanation rather * than as one run-on sentence. Both come from the database and are escaped here, as the label is rendered as HTML. * * The description is not a translatable string but a pair of columns on the list itself, so which one to show can * only be decided once the locale the form renders in is known. + * + * A subscription that is waiting to be carried to Mailman or Listmonk says so after the name, because that is why + * the checkbox beside it cannot be moved. */ final readonly class MailingListLabel implements TranslatableInterface { - public function __construct(private MailingList $list) - { + public function __construct( + private MailingList $list, + private ?TranslatableInterface $state = null, + ) { } #[Override] @@ -39,12 +44,27 @@ public function trans( ? $this->list->getEnDescription() : $this->list->getNlDescription(); + $name = htmlspecialchars( + $this->list->name, + ENT_QUOTES | ENT_SUBSTITUTE, + ); + + if (null !== $this->state) { + $name .= sprintf( + ' (%s)', + htmlspecialchars( + $this->state->trans( + $translator, + $locale, + ), + ENT_QUOTES | ENT_SUBSTITUTE, + ), + ); + } + return sprintf( '%s%s', - htmlspecialchars( - $this->list->name, - ENT_QUOTES | ENT_SUBSTITUTE, - ), + $name, htmlspecialchars( $description, ENT_QUOTES | ENT_SUBSTITUTE, diff --git a/src/Form/Database/MailingListType.php b/src/Form/Database/MailingListType.php index 4e6cc42f4..6571facb0 100644 --- a/src/Form/Database/MailingListType.php +++ b/src/Form/Database/MailingListType.php @@ -104,6 +104,19 @@ public function buildForm( ], ); + $builder->add( + 'selfService', + CheckboxType::class, + [ + 'label' => t('Members may manage their own subscription'), + 'help' => t( + // phpcs:ignore -- user-visible strings should not be split + 'Leave this off for a list that belongs to one year or one group, which is offered when somebody joins but is not a list anybody may put themselves on later.', + ), + 'required' => false, + ], + ); + $builder->add( 'mailmanList', EntityType::class, diff --git a/src/Form/Database/MemberListsType.php b/src/Form/Database/MemberListsType.php index 12386e50e..3eacc2de3 100644 --- a/src/Form/Database/MemberListsType.php +++ b/src/Form/Database/MemberListsType.php @@ -15,11 +15,15 @@ use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\Form\FormEvents; use Symfony\Component\OptionsResolver\OptionsResolver; +use Symfony\Component\Translation\TranslatableMessage; +use Symfony\Contracts\Translation\TranslatableInterface; use Symfony\Contracts\Translation\TranslatorInterface; use function array_combine; +use function array_flip; +use function array_intersect; +use function array_intersect_key; use function array_keys; -use function array_map; use function array_unique; use function array_values; use function in_array; @@ -46,18 +50,32 @@ public function buildForm( FormBuilderInterface $builder, array $options, ): void { + $describe = true === $options['describe']; $subscriptions = $this->subscriptionStates($options['member']); $locked = $this->lockedLists($subscriptions); - $listNames = array_map( - static fn (MailingList $list): string => $list->name, - $this->mailingListRepository->findAll(), + $byName = []; + + foreach ($options['lists'] ?? $this->mailingListRepository->findAll() as $list) { + $byName[$list->name] = $list; + } + + $listNames = array_keys($byName); + // A list that is not on offer is not theirs to leave either, so it is never read as an unsubscribe. + $locked = array_values(array_intersect( + $locked, + $listNames, + )); + $subscriptions = array_intersect_key( + $subscriptions, + array_flip($listNames), ); $builder->add( 'lists', ChoiceType::class, [ - 'label' => t('Lists'), + // The member's own page says what the panel around it is for, so a legend there would say it twice. + 'label' => $describe ? false : t('Lists'), 'expanded' => true, 'multiple' => true, 'required' => false, @@ -66,13 +84,31 @@ public function buildForm( $listNames, ), 'data' => array_keys($subscriptions), + 'label_html' => $describe, // Resolved while the view is built rather than at build time, so the state follows the request locale. - 'choice_label' => function (string $name) use ($subscriptions): string { - if (!isset($subscriptions[$name])) { + 'choice_label' => function ( + string $name, + ) use ( + $subscriptions, + $byName, + $describe, + ): string|TranslatableInterface { + $state = isset($subscriptions[$name]) + ? $this->stateMessage(...$subscriptions[$name]) + : null; + + if ($describe) { + return new MailingListLabel( + $byName[$name], + $state, + ); + } + + if (null === $state) { return $name; } - return $name . ' (' . $this->stateLabel(...$subscriptions[$name]) . ')'; + return $name . ' (' . $state->trans($this->translator) . ')'; }, 'choice_attr' => static function (string $name) use ($locked): array { if ( @@ -87,7 +123,8 @@ public function buildForm( return ['disabled' => true]; }, - 'choice_translation_domain' => false, + // A described label is a translatable object, and only the theme's `trans` renders one. + 'choice_translation_domain' => $describe ? null : false, ], ); @@ -128,6 +165,27 @@ public function configureOptions(OptionsResolver $resolver): void 'member', Member::class, ); + + $resolver->setDefault( + 'describe', + false, + ); + $resolver->setAllowedTypes( + 'describe', + 'bool', + ); + + $resolver->setDefault( + 'lists', + null, + ); + $resolver->setAllowedTypes( + 'lists', + [ + 'null', + MailingList::class . '[]', + ], + ); } /** @@ -173,25 +231,25 @@ private function lockedLists(array $subscriptions): array return $locked; } - private function stateLabel( + private function stateMessage( bool $toBeCreated, bool $toBeDeleted, - ): string { + ): TranslatableMessage { if ( $toBeCreated && $toBeDeleted ) { - return $this->translator->trans('email address change pending'); + return new TranslatableMessage('email address change pending'); } if ($toBeDeleted) { - return $this->translator->trans('to be deleted'); + return new TranslatableMessage('to be deleted'); } if ($toBeCreated) { - return $this->translator->trans('to be created'); + return new TranslatableMessage('to be created'); } - return $this->translator->trans('synced'); + return new TranslatableMessage('synced'); } } diff --git a/src/Form/User/MemberEmailChangeType.php b/src/Form/User/MemberEmailChangeType.php new file mode 100644 index 000000000..e2edce790 --- /dev/null +++ b/src/Form/User/MemberEmailChangeType.php @@ -0,0 +1,65 @@ +|null> + */ +class MemberEmailChangeType extends AbstractType +{ + public function __construct(private readonly LowercaseTransformer $lowercaseTransformer) + { + } + + /** + * @param array $options + */ + #[Override] + public function buildForm( + FormBuilderInterface $builder, + array $options, + ): void { + $builder->add( + 'email', + EmailType::class, + [ + 'label' => t('New e-mail address'), + 'help' => t('We send a message to this address to confirm that it reaches you.'), + 'constraints' => [ + new Assert\NotBlank(), + new Assert\Email(), + new Assert\Length(max: 255), + ], + ], + ); + + $builder->get('email')->addModelTransformer($this->lowercaseTransformer); + + $builder->add( + 'submit', + SubmitType::class, + ['label' => t('Change e-mail address')], + ); + } + + #[Override] + public function configureOptions(OptionsResolver $resolver): void + { + $resolver->setDefaults(['data_class' => null]); + } +} diff --git a/src/Message/Database/EmailChangeConfirmationEmail.php b/src/Message/Database/EmailChangeConfirmationEmail.php new file mode 100644 index 000000000..5b3652bba --- /dev/null +++ b/src/Message/Database/EmailChangeConfirmationEmail.php @@ -0,0 +1,30 @@ +lidnr; + } + + public function getNewEmail(): string + { + return $this->newEmail; + } + + public function getToken(): string + { + return $this->token; + } +} diff --git a/src/Message/Database/EmailChangedNoticeEmail.php b/src/Message/Database/EmailChangedNoticeEmail.php new file mode 100644 index 000000000..505337611 --- /dev/null +++ b/src/Message/Database/EmailChangedNoticeEmail.php @@ -0,0 +1,33 @@ +lidnr; + } + + public function getPreviousEmail(): string + { + return $this->previousEmail; + } + + public function getNewEmail(): string + { + return $this->newEmail; + } +} diff --git a/src/MessageHandler/Database/EmailChangeConfirmationEmailHandler.php b/src/MessageHandler/Database/EmailChangeConfirmationEmailHandler.php new file mode 100644 index 000000000..458e8557f --- /dev/null +++ b/src/MessageHandler/Database/EmailChangeConfirmationEmailHandler.php @@ -0,0 +1,57 @@ +memberRepository->findSimple($message->getLidnr()); + + // Gone between the asking and now: there is no longer anybody to confirm anything for. + if (null === $member) { + return; + } + + $this->emailService->send( + new Address( + $message->getNewEmail(), + $member->getFullName(), + ), + 'Confirm your e-mail address (' . $member->lidnr . ')', + 'database/email/email-change-confirm.html.twig', + [ + 'firstName' => $member->firstName, + 'newEmail' => $message->getNewEmail(), + // The message is in English, so ask for the English page rather than whatever the router holds. + 'url' => $this->urlGenerator->generate( + 'user_email_change_claim', + [ + '_locale' => Languages::English->getLangParam(), + 'token' => $message->getToken(), + ], + UrlGeneratorInterface::ABSOLUTE_URL, + ), + ], + $this->emailService->secretary(), + ); + } +} diff --git a/src/MessageHandler/Database/EmailChangedNoticeEmailHandler.php b/src/MessageHandler/Database/EmailChangedNoticeEmailHandler.php new file mode 100644 index 000000000..b9729288b --- /dev/null +++ b/src/MessageHandler/Database/EmailChangedNoticeEmailHandler.php @@ -0,0 +1,45 @@ +memberRepository->findSimple($message->getLidnr()); + + if (null === $member) { + return; + } + + $this->emailService->send( + new Address( + $message->getPreviousEmail(), + $member->getFullName(), + ), + 'Your e-mail address was changed (' . $member->lidnr . ')', + 'database/email/email-changed.html.twig', + [ + 'firstName' => $member->firstName, + 'previousEmail' => $message->getPreviousEmail(), + 'newEmail' => $message->getNewEmail(), + ], + $this->emailService->secretary(), + ); + } +} diff --git a/src/Repository/Database/ActionLinkRepository.php b/src/Repository/Database/ActionLinkRepository.php index c5bc9aad0..37325d7f0 100644 --- a/src/Repository/Database/ActionLinkRepository.php +++ b/src/Repository/Database/ActionLinkRepository.php @@ -5,6 +5,7 @@ namespace App\Repository\Database; use App\Entity\Database\ActionLink; +use App\Entity\Database\EmailChangeLink; use App\Entity\Database\Member; use App\Entity\Database\PaymentLink; use App\Entity\Database\RenewalLink; @@ -94,6 +95,31 @@ public function findRenewalBySelector(string $selector): ?RenewalLink return $qb->getQuery()->getOneOrNullResult(); } + /** + * As {@see self::findPaymentBySelector()}, for a change of e-mail address. + */ + public function findEmailChangeBySelector(string $selector): ?EmailChangeLink + { + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->select('el, m') + ->from( + EmailChangeLink::class, + 'el', + ) + ->leftJoin( + 'el.member', + 'm', + ) + ->where('el.selector = :selector'); + + $qb->setParameter( + 'selector', + $selector, + ); + + return $qb->getQuery()->getOneOrNullResult(); + } + public function findByTempHash(string $tempHash): ?ActionLink { return $this->findOneBy(['tempHash' => $tempHash]); diff --git a/src/Repository/Database/AuditAddressChangeRepository.php b/src/Repository/Database/AuditAddressChangeRepository.php new file mode 100644 index 000000000..5ea70a685 --- /dev/null +++ b/src/Repository/Database/AuditAddressChangeRepository.php @@ -0,0 +1,23 @@ + + */ +class AuditAddressChangeRepository extends ServiceEntityRepository +{ + public function __construct(ManagerRegistry $registry) + { + parent::__construct( + $registry, + AuditAddressChange::class, + ); + } +} diff --git a/src/Repository/Database/AuditEmailChangeRepository.php b/src/Repository/Database/AuditEmailChangeRepository.php new file mode 100644 index 000000000..1fc090574 --- /dev/null +++ b/src/Repository/Database/AuditEmailChangeRepository.php @@ -0,0 +1,23 @@ + + */ +class AuditEmailChangeRepository extends ServiceEntityRepository +{ + public function __construct(ManagerRegistry $registry) + { + parent::__construct( + $registry, + AuditEmailChange::class, + ); + } +} diff --git a/src/Repository/Database/EmailChangeLinkRepository.php b/src/Repository/Database/EmailChangeLinkRepository.php new file mode 100644 index 000000000..14cfd5af5 --- /dev/null +++ b/src/Repository/Database/EmailChangeLinkRepository.php @@ -0,0 +1,40 @@ + + */ +class EmailChangeLinkRepository extends ServiceEntityRepository +{ + public function __construct(ManagerRegistry $registry) + { + parent::__construct( + $registry, + EmailChangeLink::class, + ); + } + + /** + * A member who asks again supersedes what they asked before. + */ + public function removeAllForMember(Member $member): void + { + $this->createQueryBuilder('l') + ->delete() + ->where('l.member = :member') + ->setParameter( + 'member', + $member, + ) + ->getQuery() + ->execute(); + } +} diff --git a/src/Repository/Database/MailingListRepository.php b/src/Repository/Database/MailingListRepository.php index b09f003b7..55f16e9e6 100644 --- a/src/Repository/Database/MailingListRepository.php +++ b/src/Repository/Database/MailingListRepository.php @@ -106,6 +106,17 @@ public function findAllOnForm(): array ); } + /** + * @return array + */ + public function findAllSelfService(): array + { + return $this->findBy( + ['selfService' => true], + ['name' => 'ASC'], + ); + } + /** * Find all default * diff --git a/src/Service/Database/ActionLinkService.php b/src/Service/Database/ActionLinkService.php index 43914e60d..eccd75cec 100644 --- a/src/Service/Database/ActionLinkService.php +++ b/src/Service/Database/ActionLinkService.php @@ -5,6 +5,7 @@ namespace App\Service\Database; use App\Entity\Database\ActionLink; +use App\Entity\Database\EmailChangeLink; use App\Entity\Database\PaymentLink; use App\Entity\Database\RenewalLink; use App\Repository\Database\ActionLinkRepository; @@ -75,6 +76,28 @@ public function resolvePayment(string $token): ?PaymentLink return $link; } + public function resolveEmailChange(string $token): ?EmailChangeLink + { + $split = SplitToken::split($token); + + if (null === $split) { + return null; + } + + $link = $this->actionLinkRepository->findEmailChangeBySelector($split['selector']); + + if ( + null === $link + || $link->used + || $link->linkExpired() + || !$link->tokenMatches($split['verifier']) + ) { + return null; + } + + return $link; + } + public function claim(ActionLink $link): string { $tempHash = bin2hex(random_bytes(32)); diff --git a/src/Service/Database/Member.php b/src/Service/Database/Member.php index 734f455ac..fefc47ca3 100644 --- a/src/Service/Database/Member.php +++ b/src/Service/Database/Member.php @@ -5,18 +5,22 @@ namespace App\Service\Database; use App\Entity\Database\Address as AddressModel; +use App\Entity\Database\AuditAddressChange; use App\Entity\Database\AuditEntry as AuditEntryModel; use App\Entity\Database\AuditMailingListMembership; use App\Entity\Database\AuditNote as AuditNoteModel; use App\Entity\Database\AuditRenewal as AuditRenewalModel; +use App\Entity\Database\EmailChangeLink as EmailChangeLinkModel; use App\Entity\Database\Enums\AddressTypes; use App\Entity\Database\Enums\AttentionReasons; use App\Entity\Database\Enums\MailingListMemberAction; use App\Entity\Database\Enums\MailingListMemberOrigin; +use App\Entity\Database\Enums\MemberDetailAction; use App\Entity\Database\Enums\MembershipTypes; use App\Entity\Database\Enums\PostalRegions; use App\Entity\Database\Enums\ProspectiveMemberFilter; use App\Entity\Database\Enums\Studies; +use App\Entity\Database\MailingList as MailingListModel; use App\Entity\Database\MailingListMember as MailingListMemberModel; use App\Entity\Database\Member as MemberModel; use App\Entity\Database\Membership as MembershipModel; @@ -30,6 +34,7 @@ use App\Message\Database\RegistrationUpdateEmail; use App\Repository\Database\ActionLinkRepository; use App\Repository\Database\AuditEntryRepository; +use App\Repository\Database\EmailChangeLinkRepository; use App\Repository\Database\MailingListMemberRepository; use App\Repository\Database\MailingListRepository; use App\Repository\Database\MemberRepository; @@ -45,6 +50,7 @@ use function array_diff; use function array_intersect; +use function array_map; use function array_merge; use function array_unique; use function array_values; @@ -62,6 +68,7 @@ public function __construct( private readonly ActionLinkRepository $actionLinkRepository, private readonly AuditEntryRepository $auditEntryRepository, private readonly MemberRepository $memberRepository, + private readonly EmailChangeLinkRepository $emailChangeLinkRepository, private readonly ProspectiveMemberRepository $prospectiveMemberRepository, private readonly MailingListService $mailingListService, private readonly RenewalService $renewalService, @@ -761,7 +768,10 @@ public function expiration( */ public function editAddress(FormInterface $form): ?AddressModel { - return $this->persistAddressFromForm($form); + return $this->persistAddressFromForm( + $form, + MemberDetailAction::Changed, + ); } /** @@ -771,7 +781,10 @@ public function editAddress(FormInterface $form): ?AddressModel */ public function addAddress(FormInterface $form): ?AddressModel { - return $this->persistAddressFromForm($form); + return $this->persistAddressFromForm( + $form, + MemberDetailAction::Added, + ); } /** @@ -792,6 +805,15 @@ public function removeAddress( ); $this->memberRepository->removeAddress($address); + $this->auditService->persist( + AuditAddressChange::create( + $member, + $type, + MemberDetailAction::Removed, + $this->auditUser(), + ), + ); + return $member; } @@ -806,27 +828,55 @@ public function isMailingListSyncLocked(): bool return $this->mailingListService->isSyncLocked(); } - /** - * Update mailing list subscriptions of a member - */ public function subscribeLists( MemberModel $member, FormInterface $form, ): ?MemberModel { - // Check if we are performing a sync or not. - if ($this->mailingListService->isSyncLocked()) { - return null; - } - $data = $form->getData(); /** @var string[] $selectedLists */ $selectedLists = $data['lists'] ?: []; - $currentLists = $member->getMailingListMemberships()->map( - static function (MailingListMemberModel $subscription) { - return $subscription->mailingList->name; - }, - )->toArray(); + + return $this->updateSubscriptions( + $member, + $selectedLists, + array_map( + static fn (MailingListModel $list): string => $list->name, + $this->mailingListRepository->findAll(), + ), + MailingListMemberOrigin::Manual, + ); + } + + /** + * Nothing outside the candidates is changed, or a page that offers some of the lists would unsubscribe the rest. + * Returns null while a synchronisation is running. + * + * @param string[] $selectedLists + * @param string[] $candidateLists + */ + public function updateSubscriptions( + MemberModel $member, + array $selectedLists, + array $candidateLists, + MailingListMemberOrigin $origin, + ): ?MemberModel { + if ($this->mailingListService->isSyncLocked()) { + return null; + } + + $selectedLists = array_values(array_intersect( + $selectedLists, + $candidateLists, + )); + $currentLists = array_values(array_intersect( + $member->getMailingListMemberships()->map( + static function (MailingListMemberModel $subscription) { + return $subscription->mailingList->name; + }, + )->toArray(), + $candidateLists, + )); // Determine which mailing lists the member should be (un)subscribed from/to. $intersection = array_intersect( @@ -855,12 +905,17 @@ static function (MailingListMemberModel $subscription) { $list, $member, ); + + if (null === $membership) { + continue; + } + $membership->toBeDeleted = true; $this->auditService->persist( AuditMailingListMembership::create( MailingListMemberAction::Remove, - MailingListMemberOrigin::Manual, + $origin, $member, $list, $membership->email, @@ -886,7 +941,7 @@ static function (MailingListMemberModel $subscription) { $this->auditService->persist( AuditMailingListMembership::create( MailingListMemberAction::Add, - MailingListMemberOrigin::Manual, + $origin, $member, $list, $mailingListMember->email, @@ -1142,8 +1197,10 @@ private function applyMembershipChange( return $member; } - private function persistAddressFromForm(FormInterface $form): ?AddressModel - { + private function persistAddressFromForm( + FormInterface $form, + MemberDetailAction $action, + ): ?AddressModel { if (!$form->isValid()) { return null; } @@ -1153,6 +1210,19 @@ private function persistAddressFromForm(FormInterface $form): ?AddressModel $this->memberRepository->persistAddress($address); + $member = $address->getMember(); + + if (null !== $member) { + $this->auditService->persist( + AuditAddressChange::create( + $member, + $address->type, + $action, + $this->auditUser(), + ), + ); + } + return $address; } @@ -1296,6 +1366,42 @@ static function (MemberModel $a, MemberModel $b) { ]; } + /** + * Nothing is changed by asking: the register keeps the old address until the new one is confirmed. + */ + public function requestEmailChange( + MemberModel $member, + string $newEmail, + ): EmailChangeLinkModel { + $this->emailChangeLinkRepository->removeAllForMember($member); + + $link = new EmailChangeLinkModel( + $member, + $newEmail, + ); + $this->actionLinkRepository->persist($link); + + return $link; + } + + /** + * Only the address is written; the subscriptions and the audit entry are + * {@see \App\EventListener\Database\MemberEmailChangeListener}'s. + */ + public function confirmEmailChange(EmailChangeLinkModel $link): MemberModel + { + $member = $link->member; + + $member->setEmail($link->newEmail); + $member->changedOn = new DateTimeImmutable(); + + $link->used = true; + $this->actionLinkRepository->persist($link); + $this->memberRepository->persist($member); + + return $member; + } + /** * Whether this address already belongs to someone else (a member or an applicant). * diff --git a/templates/database/email/email-change-confirm.html.twig b/templates/database/email/email-change-confirm.html.twig new file mode 100644 index 000000000..77ce1ab1e --- /dev/null +++ b/templates/database/email/email-change-confirm.html.twig @@ -0,0 +1,24 @@ +{% extends 'database/email/_base.html.twig' %} + +{% block emailTitle %}Confirm your e-mail address{% endblock %} +{% block preheader %}{% endblock %} +{% block heading %}Membership notification{% endblock %} +{% block subheading %}Confirm your e-mail address{% endblock %} +{% block body %} +

Dear {{ firstName }},

+

+ You asked us to reach you at {{ newEmail }} from now on. Before we change anything, we would like to be + sure that this message arrived, so please click + here + within 24 hours to confirm. You will be asked to sign in with your GEWIS account first. +

+

+ Until you do, we keep writing to the address we have. If you did not ask for this, you can ignore this message + and nothing will change. +

+

+ Kind regards, +
The Secretary of GEWIS +

+{% endblock %} +{% block footerNote %}You receive this message because this address was entered as the new e-mail address of a GEWIS member. You can not opt-out of these emails.{% endblock %} diff --git a/templates/database/email/email-changed.html.twig b/templates/database/email/email-changed.html.twig new file mode 100644 index 000000000..5bb42fe8e --- /dev/null +++ b/templates/database/email/email-changed.html.twig @@ -0,0 +1,22 @@ +{% extends 'database/email/_base.html.twig' %} + +{% block emailTitle %}Your e-mail address was changed{% endblock %} +{% block preheader %}{% endblock %} +{% block heading %}Membership notification{% endblock %} +{% block subheading %}Your e-mail address was changed{% endblock %} +{% block body %} +

Dear {{ firstName }},

+

+ The e-mail address of your GEWIS membership was changed from {{ previousEmail }} to + {{ newEmail }}. From now on we write to the new address, and you sign in with it as well. +

+

+ This message goes to your previous address so that you hear about the change even if somebody else made it. + If that is the case, reply to this message and the secretary will put it right. +

+

+ Kind regards, +
The Secretary of GEWIS +

+{% endblock %} +{% block footerNote %}You receive this message because the e-mail address of your GEWIS membership was changed. You can not opt-out of these emails.{% endblock %} diff --git a/templates/database/mailing/list/form.html.twig b/templates/database/mailing/list/form.html.twig index 4fa695c29..b4781a418 100644 --- a/templates/database/mailing/list/form.html.twig +++ b/templates/database/mailing/list/form.html.twig @@ -34,6 +34,7 @@ {{ form_row(form.en_description) }} {{ form_row(form.onForm) }} {{ form_row(form.defaultSub) }} + {{ form_row(form.selfService) }} {# Mailman is being phased out: a list can still be shown and unbound, but a new binding is not offered. #} {% if form.mailmanList.vars.value is not empty %} diff --git a/templates/partials/member-sidebar.html.twig b/templates/partials/member-sidebar.html.twig index c06525e2b..480de5dfa 100644 --- a/templates/partials/member-sidebar.html.twig +++ b/templates/partials/member-sidebar.html.twig @@ -26,6 +26,7 @@ {{ sidebar.group('Settings'|trans) }}
    {{ sidebar.link(path('user_settings_index'), 'fa-sliders', 'General'|trans, 'general' == activeRoute) }} + {{ sidebar.link(path('user_settings_details_index'), 'fa-id-card', 'My details'|trans, 'details' == activeRoute) }} {{ sidebar.link(path('user_settings_notifications'), 'fa-bell', 'Notifications'|trans, 'notifications' == activeRoute) }} {{ sidebar.link(path('user_settings_privacy'), 'fa-user-shield', 'Privacy'|trans, 'privacy' == activeRoute) }} {{ sidebar.link(path('user_security_index'), 'fa-shield-halved', 'Security'|trans, 'security' == activeRoute) }} diff --git a/templates/user/email-change.html.twig b/templates/user/email-change.html.twig new file mode 100644 index 000000000..2c3f20f76 --- /dev/null +++ b/templates/user/email-change.html.twig @@ -0,0 +1,40 @@ +{% extends 'layout.html.twig' %} + +{% block page_title %}{{ 'Confirm your e-mail address'|trans }}{% endblock %} + +{% block contents %} +
    +
    +
    +
    +

    {{ 'Confirm your e-mail address'|trans }}

    +
    +
    + {% if link is null %} +

    + {{ 'This link no longer works. It may have been used already, or it may have been asked for more than a day ago. You can ask for a new one from your details page.'|trans }} +

    + + {{ 'My details'|trans }} + + {% else %} +

    + {{ 'From now on the association writes to %email%, and you sign in with it as well.'|trans({ + '%email%': link.newEmail, + }) }} +

    +
    + + + + {{ 'Cancel'|trans }} + +
    + {% endif %} +
    +
    +
    +
    +{% endblock %} diff --git a/templates/user/settings/details-address-remove.html.twig b/templates/user/settings/details-address-remove.html.twig new file mode 100644 index 000000000..29b59d1d4 --- /dev/null +++ b/templates/user/settings/details-address-remove.html.twig @@ -0,0 +1,35 @@ +{% extends 'layout-sidebar.html.twig' %} + +{% block page_title %}{{ 'Remove address'|trans }}{% endblock %} + +{% block sidebar_label %}{{ app.user.member.getFullName }}{% endblock %} +{% block sidebar %} + {% include 'partials/member-sidebar.html.twig' with { + 'member': app.user.member, + 'isSelf': true, + 'activeRoute': 'details', + } %} +{% endblock %} + +{% block contents %} +
    +
    +
    +
    +

    {{ 'Remove address'|trans }}

    +
    +
    +

    + {{ 'Are you sure you want to remove your %type%?'|trans({ + '%type%': addressType.getName|trans|lower, + }) }} +

    + {{ form_start(form) }} + {{ form_widget(form.submit_yes, {'attr': {'class': 'btn-danger'}}) }} + {{ form_widget(form.submit_no, {'attr': {'class': 'btn-outline-secondary'}}) }} + {{ form_end(form) }} +
    +
    +
    +
    +{% endblock %} diff --git a/templates/user/settings/details-address.html.twig b/templates/user/settings/details-address.html.twig new file mode 100644 index 000000000..146deb364 --- /dev/null +++ b/templates/user/settings/details-address.html.twig @@ -0,0 +1,41 @@ +{% extends 'layout-sidebar.html.twig' %} + +{% block page_title %}{{ addressType.getName|trans }}{% endblock %} + +{% block sidebar_label %}{{ app.user.member.getFullName }}{% endblock %} +{% block sidebar %} + {% include 'partials/member-sidebar.html.twig' with { + 'member': app.user.member, + 'isSelf': true, + 'activeRoute': 'details', + } %} +{% endblock %} + +{% block contents %} +
    +
    +
    +
    +

    {{ addressType.getName|trans }}

    +
    +
    + {{ form_start(form) }} + {{ form_row(form.country) }} + {{ form_row(form.street) }} + {{ form_row(form.number) }} + {{ form_row(form.postalCode) }} + {{ form_row(form.city) }} + {{ form_row(form.phone) }} + {{ form_widget(form.submit, { + 'label': add ? 'Add address'|trans : 'Save address'|trans, + 'attr': {'class': 'btn-gewis-primary'}, + }) }} + + {{ 'Cancel'|trans }} + + {{ form_end(form) }} +
    +
    +
    +
    +{% endblock %} diff --git a/templates/user/settings/details.html.twig b/templates/user/settings/details.html.twig new file mode 100644 index 000000000..1bb515e95 --- /dev/null +++ b/templates/user/settings/details.html.twig @@ -0,0 +1,142 @@ +{% extends 'layout-sidebar.html.twig' %} + +{% block page_title %}{{ 'My details'|trans }}{% endblock %} + +{% block sidebar_label %}{{ app.user.member.getFullName }}{% endblock %} +{% block sidebar %} + {% include 'partials/member-sidebar.html.twig' with { + 'member': app.user.member, + 'isSelf': true, + 'activeRoute': 'details', + } %} +{% endblock %} + +{% block contents %} +
    +
    +
    +
    +

    {{ 'E-mail address'|trans }}

    +
    +
    +

    + {{ 'This is the address the association writes to, and the one you sign in with.'|trans }} +

    +
    +
    {{ 'Current address'|trans }}
    +
    {{ member.email ?? '-' }}
    +
    + {{ form_start(emailForm, { + 'action': path('user_settings_details_index'), + 'method': 'POST', + }) }} + {{ form_row(emailForm.email) }} + {{ form_widget(emailForm.submit, {'attr': {'class': 'btn-gewis-primary mt-2'}}) }} + {{ form_end(emailForm) }} +
    +
    + +
    +
    +

    {{ 'Mailing lists'|trans }}

    +
    +
    + {% if selfServiceLists is empty %} +

    + {{ 'There are no mailing lists you can subscribe to yourself. The secretary can tell you which lists you are on.'|trans }} +

    + {% elseif syncLocked %} + {# While a synchronisation runs, the pending states these checkboxes write are being turned + into real ones, and an edit made in between would be lost. #} +

    + + {{ 'Your subscriptions are being synchronised. Please come back in a few minutes.'|trans }} +

    + {% else %} + {{ form_start(listsForm, { + 'action': path('user_settings_details_index'), + 'method': 'POST', + }) }} + {{ form_row(listsForm.lists) }} + {{ form_widget(listsForm.submit, {'attr': {'class': 'btn-gewis-primary mt-2'}}) }} + {{ form_end(listsForm) }} + {% endif %} +
    +
    +
    + +
    +
    +
    +

    {{ 'Addresses'|trans }}

    +
    +
    +

    + {{ 'Where you live and where your post should go. GEWIS writes to your mail address.'|trans }} +

    + {% for row in addresses %} + {% set address = row.address %} +
    +

    {{ row.type.getName|trans }}

    + {% if address is null %} +

    {{ 'Not on file.'|trans }}

    + + {{ 'Add'|trans }} + + {% else %} +

    + {{ address.street }} {{ address.number }}
    + {{ address.postalCode }} {{ address.city }}
    + {{ address.country.value|lower|title }} + {% if address.phone is not empty %} +
    {{ address.phone }} + {% endif %} +

    + + {{ 'Edit'|trans }} + + + {{ 'Remove'|trans }} + + {% endif %} +
    + {% endfor %} +
    +
    + +
    +
    +

    {{ 'What the secretary keeps'|trans }}

    +
    +
    +
    +
    {{ 'Membership number'|trans }}
    +
    {{ member.lidnr }}
    +
    {{ 'Name'|trans }}
    +
    {{ member.getFullName }}
    +
    {{ 'Study'|trans }}
    +
    {{ member.study.getName|trans }}
    +
    {{ 'Student number'|trans }}
    +
    {{ member.studentNumber ?? '-' }}
    +
    {{ 'Membership'|trans }}
    +
    + {% set membership = member.getCurrentOrLastMembership %} + {% if membership is null %} + - + {% else %} + {{ membership.type.getName|trans }} + {{ 'until'|trans }} {{ membership.endDate|date('d-m-Y') }} + {% endif %} +
    +
    +

    + {{ 'These are recorded by the secretary. Write to them if any of it is wrong.'|trans }} +

    +
    +
    +
    +
    +{% endblock %} diff --git a/tests/Entity/Database/EmailChangeLinkTest.php b/tests/Entity/Database/EmailChangeLinkTest.php new file mode 100644 index 000000000..6f3300966 --- /dev/null +++ b/tests/Entity/Database/EmailChangeLinkTest.php @@ -0,0 +1,71 @@ +member('timon@example.com'); + $link = new EmailChangeLink( + $member, + 'timon@example.org', + ); + + self::assertSame( + 'timon@example.org', + $link->newEmail, + ); + self::assertSame( + 'timon@example.com', + $link->previousEmail, + ); + self::assertSame( + 'timon@example.com', + $member->email, + ); + self::assertFalse($link->used); + } + + public function testIsGoodForADay(): void + { + $link = new EmailChangeLink( + $this->member('timon@example.com'), + 'timon@example.org', + ); + + self::assertFalse($link->linkExpired()); + self::assertEqualsWithDelta( + new DateTimeImmutable('+1 day')->getTimestamp(), + $link->getExpiresAt()->getTimestamp(), + 5, + ); + } + + public function testAcceptsAMemberWhoHadNoAddress(): void + { + $link = new EmailChangeLink( + new Member(), + 'timon@example.org', + ); + + self::assertNull($link->previousEmail); + } + + private function member(string $email): Member + { + $member = new Member(); + $member->setEmail($email); + + return $member; + } +} diff --git a/tests/Integration/Controller/User/EmailChangeControllerTest.php b/tests/Integration/Controller/User/EmailChangeControllerTest.php new file mode 100644 index 000000000..c56003653 --- /dev/null +++ b/tests/Integration/Controller/User/EmailChangeControllerTest.php @@ -0,0 +1,255 @@ +get('doctrine')->getManager('default'); + self::assertInstanceOf( + EntityManagerInterface::class, + $ledger, + ); + + $this->ledger = $ledger; + } + + public function testFollowingTheLinkAndConfirmingChangesTheAddress(): void + { + $member = $this->authenticateMember(); + $link = self::getContainer()->get(MemberService::class)->requestEmailChange( + $member, + 'confirmed@example.org', + ); + $token = (string) $link->plainToken; + + $request = $this->pushRequest(); + $controller = self::getContainer()->get(EmailChangeController::class); + + $claim = $controller->claim($token); + self::assertSame( + Response::HTTP_FOUND, + $claim->getStatusCode(), + ); + + $controller->confirm($this->requestWithTempHash( + $request, + $claim, + )); + + // The redirect that follows carries no hash: what says which change is meant is now the session. + $request->query->remove('th'); + + $page = $controller->confirm($request); + self::assertStringContainsString( + 'confirmed@example.org', + (string) $page->getContent(), + ); + + $applied = $controller->apply( + $request, + $this->currentUser(), + ); + self::assertSame( + Response::HTTP_FOUND, + $applied->getStatusCode(), + ); + self::assertSame( + 'confirmed@example.org', + $member->email, + ); + self::assertTrue($link->used); + } + + public function testALinkThatWasUsedIsRefused(): void + { + $member = $this->authenticateMember(); + $link = self::getContainer()->get(MemberService::class)->requestEmailChange( + $member, + 'confirmed@example.org', + ); + $token = (string) $link->plainToken; + $link->used = true; + $this->ledger->flush(); + + $request = $this->pushRequest(); + + $response = self::getContainer()->get(EmailChangeController::class)->claim($token); + + self::assertSame( + Response::HTTP_OK, + $response->getStatusCode(), + ); + self::assertStringContainsString( + 'no longer works', + (string) $response->getContent(), + ); + } + + public function testAnotherMemberCannotConfirmSomebodyElsesChange(): void + { + $member = $this->authenticateMember(); + $link = self::getContainer()->get(MemberService::class)->requestEmailChange( + $member, + 'confirmed@example.org', + ); + + $request = $this->pushRequest(); + $controller = self::getContainer()->get(EmailChangeController::class); + $controller->confirm($this->requestWithTempHash( + $request, + $controller->claim((string) $link->plainToken), + )); + + $this->authenticateOther($member); + + $response = $controller->apply( + $request, + $this->currentUser(), + ); + + self::assertSame( + Response::HTTP_OK, + $response->getStatusCode(), + ); + self::assertFalse($link->used); + self::assertNotSame( + 'confirmed@example.org', + $member->email, + ); + } + + private function requestWithTempHash( + Request $request, + Response $claim, + ): Request { + self::assertInstanceOf( + RedirectResponse::class, + $claim, + ); + + $query = []; + parse_str( + (string) parse_url( + $claim->getTargetUrl(), + PHP_URL_QUERY, + ), + $query, + ); + + $request->query->set( + 'th', + $query['th'] ?? '', + ); + + return $request; + } + + private function currentUser(): User + { + $user = self::getContainer()->get('security.token_storage')->getToken()?->getUser(); + self::assertInstanceOf( + User::class, + $user, + ); + + return $user; + } + + private function authenticateMember(): LedgerMember + { + $subscription = $this->ledger->getRepository(MailingListMember::class)->findOneBy(['toBeDeleted' => false]); + self::assertInstanceOf( + MailingListMember::class, + $subscription, + ); + + $member = $subscription->member; + self::assertInstanceOf( + LedgerMember::class, + $member, + ); + + $this->authenticate($member->lidnr); + + return $member; + } + + private function authenticateOther(LedgerMember $member): void + { + $other = $this->ledger->getRepository(LedgerMember::class)->createQueryBuilder('m') + ->where('m.lidnr != :lidnr') + ->setParameter( + 'lidnr', + $member->lidnr, + ) + ->setMaxResults(1) + ->getQuery() + ->getSingleResult(); + + $this->authenticate($other->lidnr); + } + + private function authenticate(int $lidnr): void + { + $user = $this->entityManager->getRepository(User::class)->find($lidnr); + self::assertInstanceOf( + User::class, + $user, + ); + + self::getContainer()->get('security.token_storage')->setToken( + new UsernamePasswordToken( + $user, + 'main', + [UserRoles::User->value], + ), + ); + } + + private function pushRequest(): Request + { + $session = self::getContainer()->get('session.factory')->createSession(); + self::assertInstanceOf( + FlashBagAwareSessionInterface::class, + $session, + ); + + $request = new Request(); + $request->setLocale('en'); + $request->setSession($session); + self::getContainer()->get('request_stack')->push($request); + + return $request; + } +} diff --git a/tests/Integration/Controller/User/MemberDetailsControllerTest.php b/tests/Integration/Controller/User/MemberDetailsControllerTest.php new file mode 100644 index 000000000..70526b4cd --- /dev/null +++ b/tests/Integration/Controller/User/MemberDetailsControllerTest.php @@ -0,0 +1,421 @@ +get('doctrine')->getManager('default'); + self::assertInstanceOf( + EntityManagerInterface::class, + $ledger, + ); + + $this->ledger = $ledger; + } + + public function testShowsWhatAMemberMayChangeAndWhatTheSecretaryKeeps(): void + { + $member = $this->authenticateMember(); + $this->pushRequest(); + + $response = $this->render(); + + self::assertSame( + Response::HTTP_OK, + $response->getStatusCode(), + ); + + $content = (string) $response->getContent(); + + self::assertStringContainsString( + (string) $member->email, + $content, + ); + self::assertStringContainsString( + (string) $member->studentNumber, + $content, + ); + + foreach ($this->ledger->getRepository(MailingList::class)->findAll() as $list) { + if ($list->selfService) { + self::assertStringContainsString( + $list->name, + $content, + ); + self::assertStringContainsString( + $list->getEnDescription(), + $content, + ); + + continue; + } + + self::assertStringNotContainsString( + 'value="' . $list->name . '"', + $content, + ); + } + + // The panel heading says what these are, so the field adds no legend of its own. + self::assertStringNotContainsString( + 'authenticateMember(); + $before = $member->email; + + $request = new Request( + request: [ + 'member_email_change' => [ + 'email' => 'somewhere-else@example.org', + '_csrf_token' => 'csrf-token', + ], + ], + server: ['HTTP_SEC_FETCH_SITE' => 'same-origin'], + ); + $request->setMethod(Request::METHOD_POST); + $this->pushRequest($request); + + $response = $this->render($request); + + self::assertSame( + Response::HTTP_FOUND, + $response->getStatusCode(), + ); + self::assertSame( + $before, + $member->email, + ); + + $link = $this->ledger->getRepository(EmailChangeLink::class)->findOneBy(['member' => $member->lidnr]); + self::assertInstanceOf( + EmailChangeLink::class, + $link, + ); + self::assertSame( + 'somewhere-else@example.org', + $link->newEmail, + ); + self::assertSame( + $before, + $link->previousEmail, + ); + } + + public function testRefusesAnAddressThatBelongsToSomebodyElse(): void + { + $member = $this->authenticateMember(); + $someoneElse = $this->ledger->getRepository(LedgerMember::class)->createQueryBuilder('m') + ->where('m.email IS NOT NULL') + ->andWhere('m.lidnr != :lidnr') + ->setParameter( + 'lidnr', + $member->lidnr, + ) + ->setMaxResults(1) + ->getQuery() + ->getSingleResult(); + + $request = new Request( + request: [ + 'member_email_change' => [ + 'email' => (string) $someoneElse->email, + '_csrf_token' => 'csrf-token', + ], + ], + server: ['HTTP_SEC_FETCH_SITE' => 'same-origin'], + ); + $request->setMethod(Request::METHOD_POST); + $this->pushRequest($request); + + $response = $this->render($request); + + // The page is rendered again with what is wrong with it, which is what a form that did not validate answers. + self::assertSame( + Response::HTTP_UNPROCESSABLE_ENTITY, + $response->getStatusCode(), + ); + self::assertEmpty( + $this->ledger->getRepository(EmailChangeLink::class)->findBy(['member' => $member->lidnr]), + ); + } + + public function testASubmissionOnlyTouchesTheListsThatWereOffered(): void + { + // Synced subscriptions: one still waiting to be carried across is locked by the form. + $member = $this->authenticate(MemberPopulationFixture::ADMIN); + $offered = []; + foreach ($this->ledger->getRepository(MailingList::class)->findAll() as $list) { + if (!$list->selfService) { + continue; + } + + $offered[] = $list->name; + } + + self::assertNotEmpty( + $offered, + 'The seed is expected to contain a list a member may manage themselves.', + ); + + $before = []; + foreach ($member->getMailingListMemberships() as $subscription) { + $before[$subscription->mailingList->name] = $subscription->toBeDeleted; + } + + $request = new Request( + request: [ + 'member_lists' => [ + 'lists' => [], + '_csrf_token' => 'csrf-token', + ], + ], + server: ['HTTP_SEC_FETCH_SITE' => 'same-origin'], + ); + $request->setMethod(Request::METHOD_POST); + $this->pushRequest($request); + + $this->render($request); + + foreach ($member->getMailingListMemberships() as $subscription) { + $name = $subscription->mailingList->name; + + if ( + in_array( + $name, + $offered, + true, + ) + ) { + self::assertTrue( + $subscription->toBeDeleted, + 'A list that was offered and left unticked is unsubscribed.', + ); + + continue; + } + + self::assertSame( + $before[$name] ?? false, + $subscription->toBeDeleted, + 'A list that was never offered is left exactly as it was.', + ); + } + } + + public function testOffersAnAddressToAddAndOneToCorrect(): void + { + $member = $this->authenticateMemberWithAnAddress(); + $this->pushRequest(); + $controller = self::getContainer()->get(MemberDetailsController::class); + $user = $this->currentUser(); + + foreach (AddressTypes::cases() as $type) { + $response = $controller->address( + new Request(), + $type, + $user, + ); + + self::assertSame( + Response::HTTP_OK, + $response->getStatusCode(), + ); + self::assertStringContainsString( + '/user/settings/details', + (string) $response->getContent(), + 'The page offers a way back to the details it belongs to.', + ); + } + + // Removing is offered for an address that is on file, and is nothing to offer for one that is not. + $onFile = $member->getAddresses()->first(); + self::assertInstanceOf( + Address::class, + $onFile, + ); + + $response = $controller->removeAddress( + new Request(), + $onFile->type, + $user, + ); + + self::assertSame( + Response::HTTP_OK, + $response->getStatusCode(), + ); + } + + private function currentUser(): User + { + $user = self::getContainer()->get('security.token_storage')->getToken()?->getUser(); + self::assertInstanceOf( + User::class, + $user, + ); + + return $user; + } + + private function render(?Request $request = null): Response + { + $user = self::getContainer()->get('security.token_storage')->getToken()?->getUser(); + self::assertInstanceOf( + User::class, + $user, + ); + + return self::getContainer()->get(MemberDetailsController::class)->index( + $request ?? new Request(), + $user, + ); + } + + private function authenticateMemberWithAnAddress(): LedgerMember + { + $address = $this->ledger->getRepository(Address::class)->createQueryBuilder('a') + ->join( + 'a.member', + 'm', + ) + ->andWhere('m.deleted = false') + ->setMaxResults(1) + ->getQuery() + ->getOneOrNullResult(); + + self::assertInstanceOf( + Address::class, + $address, + 'The seed is expected to contain a member with an address.', + ); + + $member = $address->getMember(); + self::assertInstanceOf( + LedgerMember::class, + $member, + ); + + $user = $this->entityManager->getRepository(User::class)->find($member->lidnr); + self::assertInstanceOf( + User::class, + $user, + ); + + self::getContainer()->get('security.token_storage')->setToken( + new UsernamePasswordToken( + $user, + 'main', + [UserRoles::User->value], + ), + ); + + return $member; + } + + private function authenticate(int $lidnr): LedgerMember + { + $member = $this->ledger->getRepository(LedgerMember::class)->find($lidnr); + self::assertInstanceOf( + LedgerMember::class, + $member, + ); + + $user = $this->entityManager->getRepository(User::class)->find($lidnr); + self::assertInstanceOf( + User::class, + $user, + ); + + self::getContainer()->get('security.token_storage')->setToken( + new UsernamePasswordToken( + $user, + 'main', + [UserRoles::User->value], + ), + ); + + return $member; + } + + private function authenticateMember(): LedgerMember + { + $subscription = $this->ledger->getRepository(MailingListMember::class)->findOneBy(['toBeDeleted' => false]); + self::assertInstanceOf( + MailingListMember::class, + $subscription, + ); + + $member = $subscription->member; + self::assertInstanceOf( + LedgerMember::class, + $member, + ); + + $user = $this->entityManager->getRepository(User::class)->find($member->lidnr); + self::assertInstanceOf( + User::class, + $user, + 'Every seeded member has an account.', + ); + + self::getContainer()->get('security.token_storage')->setToken( + new UsernamePasswordToken( + $user, + 'main', + [UserRoles::User->value], + ), + ); + + return $member; + } + + private function pushRequest(?Request $request = null): void + { + $session = self::getContainer()->get('session.factory')->createSession(); + self::assertInstanceOf( + FlashBagAwareSessionInterface::class, + $session, + ); + + $request ??= new Request(); + $request->setLocale('en'); + $request->setSession($session); + self::getContainer()->get('request_stack')->push($request); + } +} diff --git a/tests/Integration/EventListener/Database/MemberEmailChangeListenerTest.php b/tests/Integration/EventListener/Database/MemberEmailChangeListenerTest.php new file mode 100644 index 000000000..5f4577742 --- /dev/null +++ b/tests/Integration/EventListener/Database/MemberEmailChangeListenerTest.php @@ -0,0 +1,152 @@ +get('doctrine')->getManager('default'); + self::assertInstanceOf( + EntityManagerInterface::class, + $ledger, + ); + + $this->ledger = $ledger; + } + + public function testCarriesSubscriptionsOverToTheNewAddress(): void + { + $member = $this->subscribedMember(); + $before = $this->subscriptionsOf($member); + + self::assertNotEmpty( + $before, + 'The seed is expected to contain a member on at least one mailing list.', + ); + + $newEmail = 'carried-over@example.org'; + + $member->setEmail($newEmail); + $this->ledger->flush(); + + foreach ($before as $subscription) { + self::assertTrue( + $subscription->toBeDeleted, + 'The subscription under the address that was replaced is on its way out.', + ); + } + + $carried = []; + foreach ($this->subscriptionsOf($member) as $subscription) { + if ($newEmail !== $subscription->email) { + continue; + } + + $carried[$subscription->mailingList->name] = $subscription; + self::assertTrue($subscription->toBeCreated); + } + + foreach ($before as $subscription) { + self::assertArrayHasKey( + $subscription->mailingList->name, + $carried, + 'Every list the member was on under the old address is one they are on under the new one.', + ); + } + } + + public function testWritesTheChangeIntoTheAuditTrail(): void + { + $member = $this->subscribedMember(); + $oldEmail = $member->email; + + $member->setEmail('audited@example.org'); + $this->ledger->flush(); + + $audit = $this->ledger->getRepository(AuditEmailChange::class)->findOneBy( + ['member' => $member->lidnr], + ['id' => 'DESC'], + ); + + self::assertInstanceOf( + AuditEmailChange::class, + $audit, + ); + self::assertSame( + $oldEmail, + $audit->oldEmail, + ); + self::assertSame( + 'audited@example.org', + $audit->newEmail, + ); + } + + public function testDoesNothingWhenTheAddressIsUnchanged(): void + { + $member = $this->subscribedMember(); + $auditRepository = $this->ledger->getRepository(AuditEmailChange::class); + $before = $auditRepository->count([]); + + $member->setEmail($member->email); + $this->ledger->flush(); + + self::assertSame( + $before, + $auditRepository->count([]), + ); + } + + private function subscribedMember(): Member + { + $subscription = $this->ledger->getRepository(MailingListMember::class)->findOneBy(['toBeDeleted' => false]); + self::assertInstanceOf( + MailingListMember::class, + $subscription, + ); + + $member = $subscription->member; + self::assertInstanceOf( + Member::class, + $member, + ); + + return $member; + } + + /** + * @return MailingListMember[] + */ + private function subscriptionsOf(Member $member): array + { + $subscriptions = []; + + foreach ($member->getMailingListMemberships() as $subscription) { + if ($subscription->toBeDeleted) { + continue; + } + + $subscriptions[] = $subscription; + } + + return $subscriptions; + } +} diff --git a/tests/Integration/MessageHandler/Database/EmailChangeEmailsTest.php b/tests/Integration/MessageHandler/Database/EmailChangeEmailsTest.php new file mode 100644 index 000000000..ff9edaf3f --- /dev/null +++ b/tests/Integration/MessageHandler/Database/EmailChangeEmailsTest.php @@ -0,0 +1,147 @@ +get('doctrine')->getManager('default'); + self::assertInstanceOf( + EntityManagerInterface::class, + $ledger, + ); + + $this->ledger = $ledger; + } + + public function testTheConfirmationGoesToTheNewAddressAndCarriesAWorkingLink(): void + { + $member = $this->member(); + $link = self::getContainer()->get(MemberService::class)->requestEmailChange( + $member, + 'somewhere-else@example.org', + ); + + self::getContainer()->get(EmailChangeConfirmationEmailHandler::class)->__invoke( + new EmailChangeConfirmationEmail( + $member->lidnr, + 'somewhere-else@example.org', + (string) $link->plainToken, + ), + ); + + self::assertEmailCount(1); + $email = self::getMailerMessage(); + self::assertInstanceOf( + Email::class, + $email, + ); + + self::assertSame( + 'somewhere-else@example.org', + $email->getTo()[0]->getAddress(), + 'A confirmation is worth something only when it goes to the address being claimed.', + ); + + $body = (string) $email->getHtmlBody(); + self::assertStringContainsString( + 'somewhere-else@example.org', + $body, + ); + + // The link in the message is the credential. Following it has to lead back to this change and no other. + self::assertSame( + 1, + preg_match( + '{/en/user/email-change/([0-9a-f]{32}\.[0-9a-f]{96})}', + $body, + $matches, + ), + 'The message carries a link to the confirmation page.', + ); + self::assertSame( + $link->id, + self::getContainer()->get(ActionLinkService::class)->resolveEmailChange($matches[1])?->id, + ); + } + + public function testTheNoticeGoesToTheAddressThatWasReplaced(): void + { + $member = $this->member(); + + self::getContainer()->get(EmailChangedNoticeEmailHandler::class)->__invoke( + new EmailChangedNoticeEmail( + $member->lidnr, + 'was@example.org', + 'is-now@example.org', + ), + ); + + self::assertEmailCount(1); + $email = self::getMailerMessage(); + self::assertInstanceOf( + Email::class, + $email, + ); + + self::assertSame( + 'was@example.org', + $email->getTo()[0]->getAddress(), + ); + + $body = (string) $email->getHtmlBody(); + self::assertStringContainsString( + 'was@example.org', + $body, + ); + self::assertStringContainsString( + 'is-now@example.org', + $body, + ); + } + + private function member(): Member + { + $member = $this->ledger->getRepository(Member::class)->createQueryBuilder('m') + ->where('m.email IS NOT NULL') + ->andWhere('m.deleted = false') + ->setMaxResults(1) + ->getQuery() + ->getOneOrNullResult(); + + self::assertInstanceOf( + Member::class, + $member, + ); + + return $member; + } +} diff --git a/tests/Service/Database/ActionLinkServiceTest.php b/tests/Service/Database/ActionLinkServiceTest.php new file mode 100644 index 000000000..535d4341a --- /dev/null +++ b/tests/Service/Database/ActionLinkServiceTest.php @@ -0,0 +1,105 @@ +renewalLink(); + $token = (string) $link->plainToken; + + self::assertSame( + $link, + $this->service($link)->resolveRenewal($token), + ); + } + + public function testRefusesAVerifierItDidNotMint(): void + { + $link = $this->renewalLink(); + [$selector] = explode( + '.', + (string) $link->plainToken, + ); + + self::assertNull( + $this->service($link)->resolveRenewal($selector . '.0123456789abcdef'), + ); + } + + public function testRefusesATokenThatIsNotSplitInTwo(): void + { + $link = $this->renewalLink(); + + self::assertNull($this->service($link)->resolveRenewal('no-verifier-here')); + } + + public function testRefusesALinkThatWasFollowedAlready(): void + { + $link = $this->renewalLink(); + $link->used = true; + + self::assertNull( + $this->service($link)->resolveRenewal((string) $link->plainToken), + ); + } + + public function testRefusesAClaimHashThatIsPastItsThreeMinutes(): void + { + $link = $this->renewalLink(); + $service = $this->service($link); + + $tempHash = $service->claim($link); + self::assertSame( + $link, + $service->findByTempHash($tempHash), + ); + + $link->tempHashExpiresAt = new DateTimeImmutable('-1 second'); + + self::assertNull($service->findByTempHash($tempHash)); + } + + private function service(RenewalLink $link): ActionLinkService + { + $repository = self::createStub(ActionLinkRepository::class); + $repository->method('findRenewalBySelector')->willReturn($link); + $repository->method('findByTempHash')->willReturn($link); + + return new ActionLinkService($repository); + } + + private function renewalLink(): RenewalLink + { + $member = new Member(); + $member->addMembership( + new Membership( + $member, + MembershipTypes::Ordinary, + new DateTimeImmutable('-2 years'), + new DateTimeImmutable('+1 month'), + ), + ); + + return new RenewalLink( + $member, + new DateTimeImmutable('+1 year'), + ); + } +} diff --git a/translations/AutocompleteBundle.en.xlf b/translations/AutocompleteBundle.en.xlf new file mode 100644 index 000000000..2c900ba42 --- /dev/null +++ b/translations/AutocompleteBundle.en.xlf @@ -0,0 +1,26 @@ + + + +
    + +
    + + + Add %placeholder%... + Add %placeholder%... + + + Loading more results... + Loading more results... + + + No more results + No more results + + + No results found + No results found + + +     +     diff --git a/translations/AutocompleteBundle.nl.xlf b/translations/AutocompleteBundle.nl.xlf new file mode 100644 index 000000000..2fbb21b5b --- /dev/null +++ b/translations/AutocompleteBundle.nl.xlf @@ -0,0 +1,26 @@ + + + +
    + +
    + + + Add %placeholder%... + Voeg %placeholder% toe... + + + Loading more results... + Meer resultaten aan het laden... + + + No more results + Niet meer resultaten gevonden… + + + No results found + Geen resultaten gevonden… + + +     +     diff --git a/translations/messages.en.xlf b/translations/messages.en.xlf index 7719e96fb..6919f3a02 100644 --- a/translations/messages.en.xlf +++ b/translations/messages.en.xlf @@ -1045,6 +1045,10 @@ Add a vacancy Add a vacancy + + Add address + Add address + Add agenda point Add agenda point @@ -1105,6 +1109,10 @@ Add to calendar Add to calendar + + Added + Added + Address Address @@ -1565,6 +1573,10 @@ Are you sure you want to delete this album? Are you sure you want to delete this album? + + Are you sure you want to remove your %type%? + Are you sure you want to remove your %type%? + Are you sure you want to terminate the session on this device? Are you sure you want to terminate the session on this device? @@ -2133,6 +2145,10 @@ Change(s) of %entity% have been saved! Change(s) of %entity% have been saved! + + Changed + Changed + Changes go to your contact at GEWIS first; what is live stays live until they agree. Changes go to your contact at GEWIS first; what is live stays live until they agree. @@ -2717,6 +2733,10 @@ Current Members Current Members + + Current address + Current address + Current expiration Current expiration @@ -4093,6 +4113,10 @@ From From + + From now on the association writes to %email%, and you sign in with it as well. + From now on the association writes to %email%, and you sign in with it as well. + Frontpage Frontpage @@ -5117,6 +5141,10 @@ Leave the start empty to begin at once, and the end empty to keep it on until you turn it off. Windows may not overlap. Leave the start empty to begin at once, and the end empty to keep it on until you turn it off. Windows may not overlap. + + Leave this off for a list that belongs to one year or one group, which is offered when somebody joins but is not a list anybody may put themselves on later. + Leave this off for a list that belongs to one year or one group, which is offered when somebody joins but is not a list anybody may put themselves on later. + Let a body continue to exist. Let a body continue to exist. @@ -5569,6 +5597,10 @@ Members can go from the activity to this album and back. Members can go from the activity to this album and back. + + Members may manage their own subscription + Members may manage their own subscription + Members only Members only @@ -5589,6 +5621,10 @@ Members' accounts Members' accounts + + Membership + Membership + Membership Ends On (Expires On) Membership Ends On (Expires On) @@ -5629,6 +5665,10 @@ Membership types Membership types + + Memberships + Memberships + Memberships expiring within %days% days, and members missing information we need. Memberships expiring within %days% days, and members missing information we need. @@ -5837,6 +5877,10 @@ My Photos My Photos + + My details + My details + My photos My photos @@ -6417,6 +6461,10 @@ Not found in a known data breach. Not found in a known data breach. + + Not on file. + Not on file. + Not open yet Not open yet @@ -7853,6 +7901,10 @@ Remove Prospective Member Remove Prospective Member + + Remove address + Remove address + Remove agenda point? Remove agenda point? @@ -8289,6 +8341,10 @@ Save Translation Save Translation + + Save address + Save address + Save changes Save changes @@ -9137,6 +9193,10 @@ Student jobs Student jobs + + Student number + Student number + Study Study @@ -10069,6 +10129,10 @@ There are no labels yet. There are no labels yet. + + There are no mailing lists you can subscribe to yourself. The secretary can tell you which lists you are on. + There are no mailing lists you can subscribe to yourself. The secretary can tell you which lists you are on. + There are no notes for this member. There are no notes for this member. @@ -10341,6 +10405,10 @@ This is %source%. This is %source%. + + This is the address the association writes to, and the one you sign in with. + This is the address the association writes to, and the one you sign in with. + This is the first revision; nothing to compare against. This is the first revision; nothing to compare against. @@ -10377,6 +10445,14 @@ This label was already retired. This label was already retired. + + This link no longer works + This link no longer works + + + This link no longer works. It may have been used already, or it may have been asked for more than a day ago. You can ask for a new one from your details page. + This link no longer works. It may have been used already, or it may have been asked for more than a day ago. You can ask for a new one from your details page. + This list already has subscribers, so its structure is fixed. The closing moment and the texts can still be changed. This list already has subscribers, so its structure is fixed. The closing moment and the texts can still be changed. @@ -11253,6 +11329,10 @@ We match you with a member who already passed the course. We match you with a member who already passed the course. + + We send a message to this address to confirm that it reaches you. + We send a message to this address to confirm that it reaches you. + We use this address for everything we send you, so use one you keep after you stop studying. We use this address for everything we send you, so use one you keep after you stop studying. @@ -11481,6 +11561,10 @@ Where to find you Where to find you + + Where you live and where your post should go. GEWIS writes to your mail address. + Where you live and where your post should go. GEWIS writes to your mail address. + Which is basically the same thing. Which is basically the same thing. @@ -11813,6 +11897,14 @@ Your account was signed in from an unrecognised device: %name%. Your account was signed in from an unrecognised device: %name%. + + Your address has been removed. + Your address has been removed. + + + Your address has been saved. + Your address has been saved. + Your answer Your answer @@ -12029,6 +12121,18 @@ Your submission was not sent. Confirming below returns you to the page, where you can submit it again. Your submission was not sent. Confirming below returns you to the page, where you can submit it again. + + Your subscriptions are being synchronised. Please come back in a few minutes. + Your subscriptions are being synchronised. Please come back in a few minutes. + + + Your subscriptions could not be saved because they are being synchronised. Please try again later. + Your subscriptions could not be saved because they are being synchronised. Please try again later. + + + Your subscriptions have been saved. + Your subscriptions have been saved. + Your vacancies Your vacancies @@ -12145,6 +12249,10 @@ by hand by hand + + by the member + by the member + category category @@ -12513,6 +12621,10 @@ unreleased build unreleased build + + until + until + until %date% until %date% diff --git a/translations/messages.nl.xlf b/translations/messages.nl.xlf index ba439cf70..06d193734 100644 --- a/translations/messages.nl.xlf +++ b/translations/messages.nl.xlf @@ -1045,6 +1045,10 @@ Add a vacancy Vacature toevoegen + + Add address + Adres toevoegen + Add agenda point Agendapunt toevoegen @@ -1105,6 +1109,10 @@ Add to calendar Toevoegen aan agenda + + Added + Toegevoegd + Address Adres @@ -1565,6 +1573,10 @@ Are you sure you want to delete this album? Weet je zeker dat je dit album wilt verwijderen? + + Are you sure you want to remove your %type%? + Weet je zeker dat je je %type% wilt verwijderen? + Are you sure you want to terminate the session on this device? Weet je zeker dat je de sessie op dit apparaat wilt beëindigen? @@ -2133,6 +2145,10 @@ Change(s) of %entity% have been saved! Wijziging(en) van %entity% zijn opgeslagen! + + Changed + Gewijzigd + Changes go to your contact at GEWIS first; what is live stays live until they agree. Wijzigingen gaan eerst naar je contactpersoon bij GEWIS; wat online staat blijft staan tot die akkoord gaat. @@ -2717,6 +2733,10 @@ Current Members Huidige leden + + Current address + Huidig adres + Current expiration Huidige afloopdatum @@ -4093,6 +4113,10 @@ From Vanaf + + From now on the association writes to %email%, and you sign in with it as well. + Vanaf nu schrijft de vereniging naar %email%, en log je daar ook mee in. + Frontpage Voorpagina @@ -5117,6 +5141,10 @@ Leave the start empty to begin at once, and the end empty to keep it on until you turn it off. Windows may not overlap. Laat de starttijd leeg om direct te beginnen, en de eindtijd leeg om het aan te houden totdat je het uitzet. Vensters mogen niet overlappen. + + Leave this off for a list that belongs to one year or one group, which is offered when somebody joins but is not a list anybody may put themselves on later. + Laat dit uit voor een lijst die bij één jaar of één groep hoort, die wel wordt aangeboden bij aanmelding maar waar je jezelf later niet op mag zetten. + Let a body continue to exist. Laat een orgaan blijven voortbestaan. @@ -5569,6 +5597,10 @@ Members can go from the activity to this album and back. Leden kunnen van de activiteit naar dit album gaan en terug. + + Members may manage their own subscription + Leden mogen hun eigen inschrijving beheren + Members only Alleen leden @@ -5589,6 +5621,10 @@ Members' accounts Ledenaccounts + + Membership + Lidmaatschap + Membership Ends On (Expires On) Lidmaatschap eindigt op (verloopt op) @@ -5629,6 +5665,10 @@ Membership types Lidmaatschapstypes + + Memberships + Lidmaatschappen + Memberships expiring within %days% days, and members missing information we need. Lidmaatschappen die binnen %days% dagen verlopen, en leden bij wie gegevens ontbreken die we nodig hebben. @@ -5837,6 +5877,10 @@ My Photos Mijn foto's + + My details + Mijn gegevens + My photos Mijn foto’s @@ -6417,6 +6461,10 @@ Not found in a known data breach. Niet voorkomend in een bekend datalek. + + Not on file. + Niet bekend. + Not open yet Nog niet open @@ -7853,6 +7901,10 @@ Remove Prospective Member Verwijder toekomstig lid + + Remove address + Adres verwijderen + Remove agenda point? Agendapunt verwijderen? @@ -8289,6 +8341,10 @@ Save Translation Vertaling opslaan + + Save address + Adres opslaan + Save changes Wijzigingen opslaan @@ -9137,6 +9193,10 @@ Student jobs Studentenbanen + + Student number + Studentnummer + Study Studie @@ -10069,6 +10129,10 @@ There are no labels yet. Er zijn nog geen labels. + + There are no mailing lists you can subscribe to yourself. The secretary can tell you which lists you are on. + Er zijn geen mailinglijsten waar je jezelf op kunt zetten. De secretaris kan je vertellen op welke lijsten je staat. + There are no notes for this member. Dit lid heeft geen notities. @@ -10341,6 +10405,10 @@ This is %source%. Dit is %source%. + + This is the address the association writes to, and the one you sign in with. + Dit is het adres waar de vereniging naartoe schrijft, en waarmee je inlogt. + This is the first revision; nothing to compare against. Dit is de eerste revisie; er is niets om mee te vergelijken. @@ -10377,6 +10445,14 @@ This label was already retired. Dit label is al teruggetrokken. + + This link no longer works + Deze link werkt niet meer + + + This link no longer works. It may have been used already, or it may have been asked for more than a day ago. You can ask for a new one from your details page. + Deze link werkt niet meer. Misschien is hij al gebruikt, of is hij meer dan een dag geleden aangevraagd. Je kunt een nieuwe aanvragen op je gegevenspagina. + This list already has subscribers, so its structure is fixed. The closing moment and the texts can still be changed. Deze lijst heeft al inschrijvingen, dus de structuur ligt vast. Het sluitmoment en de teksten kunnen nog worden gewijzigd. @@ -11253,6 +11329,10 @@ We match you with a member who already passed the course. We koppelen je aan een lid dat het vak al gehaald heeft. + + We send a message to this address to confirm that it reaches you. + We sturen een bericht naar dit adres om te bevestigen dat het je bereikt. + We use this address for everything we send you, so use one you keep after you stop studying. We gebruiken dit adres voor alles wat we je sturen, dus gebruik er een die je houdt nadat je stopt met studeren. @@ -11481,6 +11561,10 @@ Where to find you Waar je te vinden bent + + Where you live and where your post should go. GEWIS writes to your mail address. + Waar je woont en waar je post naartoe moet. GEWIS schrijft naar je postadres. + Which is basically the same thing. Wat in feite hetzelfde is. @@ -11813,6 +11897,14 @@ Your account was signed in from an unrecognised device: %name%. Er is op je account ingelogd vanaf een onbekend apparaat: %name%. + + Your address has been removed. + Je adres is verwijderd. + + + Your address has been saved. + Je adres is opgeslagen. + Your answer Jouw antwoord @@ -12029,6 +12121,18 @@ Your submission was not sent. Confirming below returns you to the page, where you can submit it again. Je inzending is niet verstuurd. Als je hieronder bevestigt kom je terug op de pagina, waar je hem opnieuw kunt indienen. + + Your subscriptions are being synchronised. Please come back in a few minutes. + Je inschrijvingen worden gesynchroniseerd. Kom over een paar minuten terug. + + + Your subscriptions could not be saved because they are being synchronised. Please try again later. + Je inschrijvingen konden niet worden opgeslagen omdat ze worden gesynchroniseerd. Probeer het later opnieuw. + + + Your subscriptions have been saved. + Je inschrijvingen zijn opgeslagen. + Your vacancies Je vacatures @@ -12145,6 +12249,10 @@ by hand handmatig + + by the member + door het lid + category categorie @@ -12513,6 +12621,10 @@ unreleased build niet-uitgebrachte build + + until + tot + until %date% tot %date% From 9da5f209a19c8559568f9e6d6baca75df71362c6 Mon Sep 17 00:00:00 2001 From: Tom Udding Date: Sun, 30 Aug 2026 14:57:38 +0200 Subject: [PATCH 2/5] feat(database): offer a member whose membership is ending to become a graduate Closes GH-46 --- AGENTS.md | 10 + config/packages/messenger.yaml | 1 + config/routes.yaml | 21 ++ migrations/database/Version20260923140502.php | 31 ++ ...eckMembershipGraduateConversionCommand.php | 55 +++ .../Database/ProspectiveMemberController.php | 138 +++++++- src/Controller/User/EmailChangeController.php | 2 +- .../User/MemberDetailsController.php | 6 +- .../Mailing/MailingListMembershipFixture.php | 2 +- src/DataFixtures/Member/MemberFixture.php | 43 +++ src/Entity/Database/ActionLink.php | 1 + src/Entity/Database/AuditEmailChange.php | 2 +- .../Enums/GraduateConversionOutcome.php | 41 +++ .../Database/GraduateConversionLink.php | 64 ++++ src/Entity/Database/Member.php | 2 +- src/Entity/User/Enums/ApiPermissions.php | 2 +- src/Form/Database/MemberEditType.php | 2 +- .../Database/MemberGraduateConversionType.php | 129 ++++++++ src/Form/Database/MemberRenewalType.php | 4 +- .../Registration/PersonalStepType.php | 2 +- .../Registration/RegistrationData.php | 2 +- src/Form/User/MemberEmailChangeType.php | 4 +- .../Database/GraduateRemovalRequested.php | 17 + .../EmailChangeConfirmationEmailHandler.php | 2 +- .../EmailChangedNoticeEmailHandler.php | 2 +- .../GraduateRemovalRequestedHandler.php | 57 ++++ src/Repository/Checker/MemberRepository.php | 83 +++++ .../Database/ActionLinkRepository.php | 26 ++ .../GraduateConversionLinkRepository.php | 65 ++++ src/Service/Checker/Renewal.php | 66 ++++ src/Service/Database/ActionLinkService.php | 34 ++ src/Service/Database/Member.php | 74 +++++ .../Database/DeliverableEmailAddress.php | 2 +- src/Validator/Database/UnusedEmailAddress.php | 2 +- .../Database/MemberOverview.html.twig | 6 +- .../ProspectiveMemberOverview.html.twig | 4 +- .../email/email-change-confirm.html.twig | 6 +- .../database/email/email-changed.html.twig | 8 +- .../email/graduate-conversion.html.twig | 30 ++ .../graduate-removal-requested.html.twig | 22 ++ .../email/member-registration.html.twig | 2 +- .../database/join/checkout-status.html.twig | 8 +- .../database/join/graduate-declined.html.twig | 25 ++ .../database/join/graduate-done.html.twig | 16 + .../join/graduate-unavailable.html.twig | 17 + templates/database/join/graduate.html.twig | 81 +++++ .../join/prospective-member/show.html.twig | 2 +- templates/database/join/renew-done.html.twig | 2 +- templates/database/join/subscribe.html.twig | 2 +- .../member/attention-needed.html.twig | 3 + templates/database/member/show.html.twig | 2 +- templates/user/email-change.html.twig | 4 +- templates/user/settings/details.html.twig | 6 +- .../Database/GraduateConversionTest.php | 313 ++++++++++++++++++ .../GraduateRemovalRequestedHandlerTest.php | 94 ++++++ .../Checker/GraduateConversionSweepTest.php | 140 ++++++++ tests/Scheduler/MainScheduleTest.php | 1 + translations/messages.en.xlf | 194 +++++++++-- translations/messages.nl.xlf | 178 ++++++++-- translations/validators.en.xlf | 6 +- translations/validators.nl.xlf | 4 +- 61 files changed, 2056 insertions(+), 114 deletions(-) create mode 100644 migrations/database/Version20260923140502.php create mode 100644 src/Command/Checker/CheckMembershipGraduateConversionCommand.php create mode 100644 src/Entity/Database/Enums/GraduateConversionOutcome.php create mode 100644 src/Entity/Database/GraduateConversionLink.php create mode 100644 src/Form/Database/MemberGraduateConversionType.php create mode 100644 src/Message/Database/GraduateRemovalRequested.php create mode 100644 src/MessageHandler/Database/GraduateRemovalRequestedHandler.php create mode 100644 src/Repository/Database/GraduateConversionLinkRepository.php create mode 100644 templates/database/email/graduate-conversion.html.twig create mode 100644 templates/database/email/graduate-removal-requested.html.twig create mode 100644 templates/database/join/graduate-declined.html.twig create mode 100644 templates/database/join/graduate-done.html.twig create mode 100644 templates/database/join/graduate-unavailable.html.twig create mode 100644 templates/database/join/graduate.html.twig create mode 100644 tests/Integration/Controller/Database/GraduateConversionTest.php create mode 100644 tests/Integration/MessageHandler/Database/GraduateRemovalRequestedHandlerTest.php create mode 100644 tests/Integration/Service/Checker/GraduateConversionSweepTest.php diff --git a/AGENTS.md b/AGENTS.md index bd5265324..2052078f5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -462,6 +462,16 @@ Two extraction traps: `gewiswebgewis`; the register is reached with the same account, `8000` holding both `ROLE_ADMIN` and `ROLE_DATABASE_ADMIN` while `8001` holds only the former and `8002` only the latter. (Migrations have already run by the time you get here — they fire one-shot on container start.) +- **Walking the member flows by hand.** The seed is set up so every one of them can be reached without editing rows. + Signed in as `8000`: **My details** (`/en/user/settings/details`) shows the addresses (a home one is on file, the + other two are not, so adding, correcting and removing are all reachable) and the mailing lists — `8000` is on + `activities`, which a member manages, and on `announcements`, which they do not and which therefore stays off the + page. Changing the address there sends the confirmation to MailPit; following its link and confirming moves the + subscriptions with it and writes to the address that was replaced. The two sweeps have a member each, expiring + twenty days after the seed was loaded: `make sf c=check:membership:conversion:graduate` writes to + `conversiontarget@example.com` and `make sf c=check:membership:renewal:graduate` to `renewaltarget@example.com`; + both messages carry a link that answers. `make sf c=check:membership:consistency` mails the membership report, + which finds nothing until a membership is made to overlap by hand. - `make bash` — shell into the FrankenPHP `app` container; `make exec cmd="..."` runs a single command in it. - `make sf c=...` / `make composer c='...'` — the console and Composer inside the container. - `make cc` — clear the cache and restart the worker. diff --git a/config/packages/messenger.yaml b/config/packages/messenger.yaml index 6140825fd..19e3e113d 100644 --- a/config/packages/messenger.yaml +++ b/config/packages/messenger.yaml @@ -149,6 +149,7 @@ framework: # that it was, are both worth little late. 'App\Message\Database\EmailChangeConfirmationEmail': high_priority 'App\Message\Database\EmailChangedNoticeEmail': high_priority + 'App\Message\Database\GraduateRemovalRequested': normal_priority 'App\Message\User\ExportUserDataMessage': bulk 'App\Message\User\RevokeSessionsRealtimeMessage': high_priority # A security notice is worth little late, and it must not delay whatever raised it. diff --git a/config/routes.yaml b/config/routes.yaml index 37b6e4aad..12cdf2856 100644 --- a/config/routes.yaml +++ b/config/routes.yaml @@ -220,6 +220,27 @@ join_renew: _locale: '%kernel.default_locale%' methods: [GET, POST] +# Where the link in the ending-membership email points, and the page it leads to. The same two steps as the renewal +# above: the token is exchanged for a hash, and the page that asks is served at an address that contains nothing. +join_graduate_claim: + path: '/{_locale}/graduate/{token}' + controller: App\Controller\Database\ProspectiveMemberController::graduateClaim + requirements: + _locale: '%app.locales%' + token: '[0-9a-f]{32}\.[0-9a-f]{96}' + defaults: + _locale: '%kernel.default_locale%' + methods: [GET] + +join_graduate: + path: '/{_locale}/graduate' + controller: App\Controller\Database\ProspectiveMemberController::graduate + requirements: + _locale: '%app.locales%' + defaults: + _locale: '%kernel.default_locale%' + methods: [GET, POST] + # GET only, for the same reason as the sign-up address above: the form submits to the address the page is served at. join_renew_legacy_short: path: /renew/{token} diff --git a/migrations/database/Version20260923140502.php b/migrations/database/Version20260923140502.php new file mode 100644 index 000000000..596826529 --- /dev/null +++ b/migrations/database/Version20260923140502.php @@ -0,0 +1,31 @@ +addSql('ALTER TABLE ActionLink ADD outcome VARCHAR(255) DEFAULT NULL'); + } + + public function down(Schema $schema): void + { + $this->addSql('ALTER TABLE ActionLink DROP outcome'); + } +} diff --git a/src/Command/Checker/CheckMembershipGraduateConversionCommand.php b/src/Command/Checker/CheckMembershipGraduateConversionCommand.php new file mode 100644 index 000000000..8be0f9773 --- /dev/null +++ b/src/Command/Checker/CheckMembershipGraduateConversionCommand.php @@ -0,0 +1,55 @@ +runExclusively( + $output, + fn (): int => $this->executeExclusively( + $input, + $output, + ), + ); + } + + private function executeExclusively( + InputInterface $input, + OutputInterface $output, + ): int { + $this->renewalService->sendGraduateConversions(); + + return Command::SUCCESS; + } +} diff --git a/src/Controller/Database/ProspectiveMemberController.php b/src/Controller/Database/ProspectiveMemberController.php index 9f3743131..ab0a6b6cb 100644 --- a/src/Controller/Database/ProspectiveMemberController.php +++ b/src/Controller/Database/ProspectiveMemberController.php @@ -9,11 +9,15 @@ use App\Controller\Application\NoLeakHeadersTrait; use App\Controller\Application\RendersRejectedSubmissionTrait; use App\Entity\Database\Enums\MembershipTypes; +use App\Entity\Database\GraduateConversionLink; use App\Entity\Database\RenewalLink; use App\Form\Database\MemberApproveType; +use App\Form\Database\MemberGraduateConversionType; use App\Form\Database\MemberRenewalType; use App\Form\Database\Registration\RegistrationData; use App\Form\Database\Registration\RegistrationFlowType; +use App\Form\SubmitButtons; +use App\Repository\Database\GraduateConversionLinkRepository; use App\Repository\Database\RenewalLinkRepository; use App\Security\User\SudoVoter; use App\Service\Application\LocalePreference; @@ -56,6 +60,8 @@ final class ProspectiveMemberController extends AbstractController private const string RENEWAL_SESSION_KEY = '_renewal_link_id'; + private const string CONVERSION_SESSION_KEY = '_graduate_conversion_link_id'; + public function __construct( private readonly MemberService $memberService, private readonly RegistrationService $registrationService, @@ -63,6 +69,7 @@ public function __construct( private readonly LocalePreference $localePreference, private readonly ActionLinkService $actionLinkService, private readonly RenewalLinkRepository $renewalLinkRepository, + private readonly GraduateConversionLinkRepository $graduateConversionLinkRepository, ) { } @@ -113,7 +120,7 @@ public function subscribe(Request $request): Response $flow->movePrevious(RegistrationData::STEP_PERSONAL); $this->addFlash( 'error', - $this->translator->trans('There already is a member with this e-mail address.'), + $this->translator->trans('There already is a member with this email address.'), ); return $this->redirectToRoute( @@ -261,7 +268,7 @@ public function renew(Request $request): Response ) ) { $form->get('email')->addError(new FormError( - $this->translator->trans('There already is a member with this e-mail address.'), + $this->translator->trans('There already is a member with this email address.'), )); } elseif ($form->isValid()) { $this->memberService->renewMember( @@ -288,6 +295,133 @@ public function renew(Request $request): Response )); } + public function graduateClaim(string $token): Response + { + $link = $this->actionLinkService->resolveGraduateConversion($token); + + if (null === $link) { + return $this->render('database/join/graduate-unavailable.html.twig'); + } + + return $this->withNoLeakHeaders($this->redirectToRoute( + 'join_graduate', + ['th' => $this->actionLinkService->claim($link)], + )); + } + + public function graduate(Request $request): Response + { + $session = $request->getSession(); + + if (null !== ($tempHash = $request->query->get('th'))) { + $link = $this->actionLinkService->findByTempHash((string) $tempHash); + + if (!$link instanceof GraduateConversionLink) { + return $this->render('database/join/graduate-unavailable.html.twig'); + } + + // Single-use: spent as the form is handed over rather than once it is submitted. + $this->actionLinkService->consumeTempHash($link); + + $session->set( + self::CONVERSION_SESSION_KEY, + $link->id, + ); + + return $this->withNoLeakHeaders($this->redirectToRoute('join_graduate')); + } + + $linkId = $session->get(self::CONVERSION_SESSION_KEY); + + if (!is_int($linkId)) { + return $this->render('database/join/graduate-unavailable.html.twig'); + } + + $link = $this->graduateConversionLinkRepository->find($linkId); + + // Decided afresh: it may have been answered in another tab, or gone stale while the page was open. + if ( + null === $link + || $link->used + || $link->linkExpired() + ) { + $session->remove(self::CONVERSION_SESSION_KEY); + + return $this->render('database/join/graduate-unavailable.html.twig'); + } + + $member = $link->member; + $form = $this->createForm( + MemberGraduateConversionType::class, + $member, + ['conversion_link' => $link], + ); + $form->handleRequest($request); + + if ( + $form->isSubmitted() + && $form->isValid() + ) { + $declined = SubmitButtons::clicked( + $form, + 'decline', + ); + + if ($declined) { + $removal = true === $form->get('removal')->getData(); + $this->memberService->declineGraduateConversion( + $member, + $link, + $removal, + ); + + $session->remove(self::CONVERSION_SESSION_KEY); + + return $this->render( + 'database/join/graduate-declined.html.twig', + [ + 'member' => $member, + 'removal_requested' => $removal, + 'expiration' => $link->currentExpiration, + ], + ); + } + + $email = (string) $form->get('email')->getData(); + + if ( + !$this->memberService->emailBelongsToSomeoneElse( + $email, + $member, + ) + ) { + $this->memberService->acceptGraduateConversion( + $member, + $link, + ); + + $session->remove(self::CONVERSION_SESSION_KEY); + + return $this->render( + 'database/join/graduate-done.html.twig', + ['member' => $member], + ); + } + + $form->get('email')->addError(new FormError( + $this->translator->trans('There already is a member with this email address.'), + )); + } + + return $this->withNoLeakHeaders($this->render( + 'database/join/graduate.html.twig', + [ + 'form' => $form, + 'expiration' => $link->currentExpiration, + ], + )); + } + #[IsGranted(SudoVoter::ATTRIBUTE)] #[Route( path: '/members/prospective', diff --git a/src/Controller/User/EmailChangeController.php b/src/Controller/User/EmailChangeController.php index 7643d7813..147c48a9f 100644 --- a/src/Controller/User/EmailChangeController.php +++ b/src/Controller/User/EmailChangeController.php @@ -162,7 +162,7 @@ public function apply( $this->addFlash( AlertTypes::Success->value, - $this->translator->trans('Your e-mail address has been changed. You now sign in with it as well.'), + $this->translator->trans('Your email address has been changed. You now sign in with it as well.'), ); return $this->redirectToRoute('user_settings_details_index'); diff --git a/src/Controller/User/MemberDetailsController.php b/src/Controller/User/MemberDetailsController.php index 1e3f3e76d..e411a4fd6 100644 --- a/src/Controller/User/MemberDetailsController.php +++ b/src/Controller/User/MemberDetailsController.php @@ -323,7 +323,7 @@ private function handleEmailChange( if ($email === $member->email) { $form->get('email')->addError(new FormError( - $this->translator->trans('This is already your e-mail address.'), + $this->translator->trans('This is already your email address.'), )); return null; @@ -338,7 +338,7 @@ private function handleEmailChange( ) ) { $form->get('email')->addError(new FormError( - $this->translator->trans('There already is a member with this e-mail address.'), + $this->translator->trans('There already is a member with this email address.'), )); return null; @@ -361,7 +361,7 @@ private function handleEmailChange( AlertTypes::Info->value, $this->translator->trans( // phpcs:ignore -- user-visible strings should not be split - 'We have sent a message to your new e-mail address. Your address changes once you have followed the link in it.', + 'We have sent a message to your new email address. Your address changes once you have followed the link in it.', ), ); diff --git a/src/DataFixtures/Mailing/MailingListMembershipFixture.php b/src/DataFixtures/Mailing/MailingListMembershipFixture.php index 3643b2d9c..d02e23826 100644 --- a/src/DataFixtures/Mailing/MailingListMembershipFixture.php +++ b/src/DataFixtures/Mailing/MailingListMembershipFixture.php @@ -116,7 +116,7 @@ private function subscribe( $membership = new MailingListMember(); $membership->mailingList = $list; $membership->setMember($member); - $membership->email = $member->email ?? throw new LogicException('The seeded member has no e-mail address.'); + $membership->email = $member->email ?? throw new LogicException('The seeded member has no email address.'); return $membership; } diff --git a/src/DataFixtures/Member/MemberFixture.php b/src/DataFixtures/Member/MemberFixture.php index 1e0ccaa24..cd2e1c72f 100644 --- a/src/DataFixtures/Member/MemberFixture.php +++ b/src/DataFixtures/Member/MemberFixture.php @@ -51,6 +51,8 @@ class MemberFixture extends Fixture implements FixtureGroupInterface public const string REF_MEMBER_ATTN_EXTERNAL_ACTIVE = 'attn-external-active'; public const string REF_MEMBER_ATTN_GRADUATE_ACTIVE = 'attn-graduate-active'; public const string REF_MEMBER_ATTN_MISCLASSIFIED = 'attn-misclassified'; + public const string REF_MEMBER_CONVERSION_TARGET = 'attn-conversion-target'; + public const string REF_MEMBER_RENEWAL_TARGET = 'attn-renewal-target'; #[Override] public function load(ObjectManager $manager): void @@ -563,6 +565,47 @@ private function loadAttentionMembers(ObjectManager $manager): void joinDay: 1, ), ); + + // Twenty days from whenever the seed was loaded, so both sweeps always have somebody to write to. + $expiringSoon = new DateTimeImmutable()->modify('+20 days'); + + $this->addReference( + self::REF_MEMBER_CONVERSION_TARGET, + $this->makeAttentionMember( + $manager, + 'C.', + 'ÅTTN-F1', + 'ConversionTarget', + 23, + 'conversiontarget@example.com', + $this->studentNumber(19), + Studies::BAM, + $this->associationYearChain( + 4, + $expiringSoon, + ), + ), + ); + + $this->addReference( + self::REF_MEMBER_RENEWAL_TARGET, + $this->makeAttentionMember( + $manager, + 'R.', + 'ÅTTN-F2', + 'RenewalTarget', + 31, + 'renewaltarget@example.com', + $this->studentNumber(20), + Studies::None, + $this->associationYearChain( + 5, + $expiringSoon, + MembershipTypes::Graduate, + 2, + ), + ), + ); } /** diff --git a/src/Entity/Database/ActionLink.php b/src/Entity/Database/ActionLink.php index 41781e9d1..3ad941f9b 100644 --- a/src/Entity/Database/ActionLink.php +++ b/src/Entity/Database/ActionLink.php @@ -36,6 +36,7 @@ 'payment' => PaymentLink::class, 'renewal' => RenewalLink::class, 'email_change' => EmailChangeLink::class, + 'graduate_conversion' => GraduateConversionLink::class, ], )] #[Index( diff --git a/src/Entity/Database/AuditEmailChange.php b/src/Entity/Database/AuditEmailChange.php index fb3897195..1733e014a 100644 --- a/src/Entity/Database/AuditEmailChange.php +++ b/src/Entity/Database/AuditEmailChange.php @@ -14,7 +14,7 @@ class AuditEmailChange extends AuditEntry { protected const bool IMMUTABLE = true; - private const string BODY_FORMAT = 'Changed e-mail address of %s from ' + private const string BODY_FORMAT = 'Changed email address of %s from ' . '%s to %s'; #[Column( diff --git a/src/Entity/Database/Enums/GraduateConversionOutcome.php b/src/Entity/Database/Enums/GraduateConversionOutcome.php new file mode 100644 index 000000000..b762be977 --- /dev/null +++ b/src/Entity/Database/Enums/GraduateConversionOutcome.php @@ -0,0 +1,41 @@ + new TranslatableMessage('Not answered'), + self::Accepted => new TranslatableMessage('Accepted'), + self::Declined => new TranslatableMessage('Declined'), + self::RemovalRequested => new TranslatableMessage('Declined, removal requested'), + self::Superseded => new TranslatableMessage('Settled by the secretary'), + }; + } + + #[Override] + public function trans( + TranslatorInterface $translator, + ?string $locale = null, + ): string { + return $this->getName()->trans( + $translator, + $locale, + ); + } +} diff --git a/src/Entity/Database/GraduateConversionLink.php b/src/Entity/Database/GraduateConversionLink.php new file mode 100644 index 000000000..d02608712 --- /dev/null +++ b/src/Entity/Database/GraduateConversionLink.php @@ -0,0 +1,64 @@ +member = $member; + $this->currentExpiration = $currentExpiration; + $this->requestedOn = new DateTimeImmutable(); + } + + #[Override] + public function linkExpired(): bool + { + $diff = new DateTimeImmutable()->diff($this->currentExpiration); + + return 1 === $diff->invert && $diff->days > self::GRACE_DAYS; + } +} diff --git a/src/Entity/Database/Member.php b/src/Entity/Database/Member.php index 28facbc1a..73c5d4270 100644 --- a/src/Entity/Database/Member.php +++ b/src/Entity/Database/Member.php @@ -281,7 +281,7 @@ static function ($key, MailingListMember $list) use ($newEmail) { if ($mailAddressExists) { throw new RuntimeException( // phpcs:ignore -- user-visible strings should not be split - 'The e-mail address cannot be updated while there are already (pending) registrations for this member using this email address. Please try again once all list updates have been processed.', + 'The email address cannot be updated while there are already (pending) registrations for this member using this email address. Please try again once all list updates have been processed.', ); } diff --git a/src/Entity/User/Enums/ApiPermissions.php b/src/Entity/User/Enums/ApiPermissions.php index ad55ccbad..a45c0467a 100644 --- a/src/Entity/User/Enums/ApiPermissions.php +++ b/src/Entity/User/Enums/ApiPermissions.php @@ -70,7 +70,7 @@ public function getName(): TranslatableMessage self::KeyholdersR => new TranslatableMessage('Keys - List keyholders'), self::MailingListsR => new TranslatableMessage('Mailing lists - List mailing lists'), self::MailingListMembersR => new TranslatableMessage( - 'Mailing lists - List subscribers of a mailing list, including their e-mail addresses', + 'Mailing lists - List subscribers of a mailing list, including their email addresses', ), self::ActivitiesR => new TranslatableMessage('Activities - List activities'), self::PhotoAlbumsR => new TranslatableMessage('Photos - Read albums and their photos'), diff --git a/src/Form/Database/MemberEditType.php b/src/Form/Database/MemberEditType.php index 84f126a40..cff25bf8f 100644 --- a/src/Form/Database/MemberEditType.php +++ b/src/Form/Database/MemberEditType.php @@ -119,7 +119,7 @@ public function buildForm( 'email', EmailType::class, [ - 'label' => t('E-mail Address'), + 'label' => t('Email Address'), 'required' => false, 'empty_data' => null, 'constraints' => [new Assert\Email()], diff --git a/src/Form/Database/MemberGraduateConversionType.php b/src/Form/Database/MemberGraduateConversionType.php new file mode 100644 index 000000000..99201fa7e --- /dev/null +++ b/src/Form/Database/MemberGraduateConversionType.php @@ -0,0 +1,129 @@ + + */ +class MemberGraduateConversionType extends AbstractType +{ + /** + * @param array $options + */ + #[Override] + public function buildForm( + FormBuilderInterface $builder, + array $options, + ): void { + $builder->add( + 'lastName', + TextType::class, + self::fixedOptions(t('Last Name')), + ); + $builder->add( + 'middleName', + TextType::class, + self::fixedOptions(t('Last Name Prepositional Particle')), + ); + $builder->add( + 'initials', + TextType::class, + self::fixedOptions(t('Initial(s)')), + ); + $builder->add( + 'firstName', + TextType::class, + self::fixedOptions(t('First Name')), + ); + + $builder->add( + 'email', + EmailType::class, + [ + 'label' => t('Email Address'), + 'help' => t('Use an address you keep after you stop studying.'), + 'constraints' => [new Assert\NotBlank()], + ], + ); + + $builder->add( + 'supremum', + CheckboxType::class, + [ + 'label' => t('I\'d like to receive the Supremum magazine 3 times a year'), + 'required' => false, + ], + ); + + $builder->add( + 'accept', + SubmitType::class, + ['label' => t('Stay on as a graduate')], + ); + + $builder->add( + 'decline', + SubmitType::class, + ['label' => t('End my membership')], + ); + + // The secretary acts on this: the register is not something a member deletes themselves. + $builder->add( + 'removal', + CheckboxType::class, + [ + 'label' => t('Also ask the secretary to remove my data'), + 'mapped' => false, + 'required' => false, + ], + ); + + $builder->get('email')->addModelTransformer(new LowercaseTransformer()); + $builder->get('supremum')->addModelTransformer(new OptInTransformer()); + } + + #[Override] + public function configureOptions(OptionsResolver $resolver): void + { + $resolver->setDefaults(['data_class' => Member::class]); + + $resolver->setRequired('conversion_link'); + $resolver->setAllowedTypes( + 'conversion_link', + GraduateConversionLink::class, + ); + } + + /** + * @return array + */ + private static function fixedOptions(TranslatableMessage $label): array + { + return [ + 'label' => $label, + 'disabled' => true, + 'attr' => ['readonly' => true], + ]; + } +} diff --git a/src/Form/Database/MemberRenewalType.php b/src/Form/Database/MemberRenewalType.php index c3e96eee9..d185369b4 100644 --- a/src/Form/Database/MemberRenewalType.php +++ b/src/Form/Database/MemberRenewalType.php @@ -62,7 +62,7 @@ public function buildForm( 'email', EmailType::class, [ - 'label' => t('E-mail Address'), + 'label' => t('Email Address'), 'constraints' => [new Assert\NotBlank()], ], ); @@ -76,7 +76,7 @@ public function buildForm( [ 'widget' => 'single_text', 'mapped' => false, - 'data' => $options['renewal_link']->getNewExpiration(), + 'data' => $options['renewal_link']->newExpiration, ], ), ); diff --git a/src/Form/Database/Registration/PersonalStepType.php b/src/Form/Database/Registration/PersonalStepType.php index 78d813788..d37b3008c 100644 --- a/src/Form/Database/Registration/PersonalStepType.php +++ b/src/Form/Database/Registration/PersonalStepType.php @@ -56,7 +56,7 @@ public function buildForm( ->add( 'email', EmailType::class, - ['label' => t('E-mail Address')], + ['label' => t('Email Address')], ) ->add( 'birth', diff --git a/src/Form/Database/Registration/RegistrationData.php b/src/Form/Database/Registration/RegistrationData.php index 70fc10cf9..9e5a88b00 100644 --- a/src/Form/Database/Registration/RegistrationData.php +++ b/src/Form/Database/Registration/RegistrationData.php @@ -72,7 +72,7 @@ final class RegistrationData #[Assert\Regex( pattern: '/\.tue\.nl$/i', // phpcs:ignore -- user-visible strings should not be split - message: 'You cannot use your TU/e (student) e-mail address because if you leave or stop studying, we can no longer reach you about important announcements.', + message: 'You cannot use your TU/e (student) email address because if you leave or stop studying, we can no longer reach you about important announcements.', match: false, groups: [self::STEP_PERSONAL], )] diff --git a/src/Form/User/MemberEmailChangeType.php b/src/Form/User/MemberEmailChangeType.php index e2edce790..b9aae8217 100644 --- a/src/Form/User/MemberEmailChangeType.php +++ b/src/Form/User/MemberEmailChangeType.php @@ -38,7 +38,7 @@ public function buildForm( 'email', EmailType::class, [ - 'label' => t('New e-mail address'), + 'label' => t('New email address'), 'help' => t('We send a message to this address to confirm that it reaches you.'), 'constraints' => [ new Assert\NotBlank(), @@ -53,7 +53,7 @@ public function buildForm( $builder->add( 'submit', SubmitType::class, - ['label' => t('Change e-mail address')], + ['label' => t('Change email address')], ); } diff --git a/src/Message/Database/GraduateRemovalRequested.php b/src/Message/Database/GraduateRemovalRequested.php new file mode 100644 index 000000000..ed97841fe --- /dev/null +++ b/src/Message/Database/GraduateRemovalRequested.php @@ -0,0 +1,17 @@ +lidnr; + } +} diff --git a/src/MessageHandler/Database/EmailChangeConfirmationEmailHandler.php b/src/MessageHandler/Database/EmailChangeConfirmationEmailHandler.php index 458e8557f..263793c39 100644 --- a/src/MessageHandler/Database/EmailChangeConfirmationEmailHandler.php +++ b/src/MessageHandler/Database/EmailChangeConfirmationEmailHandler.php @@ -36,7 +36,7 @@ public function __invoke(EmailChangeConfirmationEmail $message): void $message->getNewEmail(), $member->getFullName(), ), - 'Confirm your e-mail address (' . $member->lidnr . ')', + 'Confirm your email address (' . $member->lidnr . ')', 'database/email/email-change-confirm.html.twig', [ 'firstName' => $member->firstName, diff --git a/src/MessageHandler/Database/EmailChangedNoticeEmailHandler.php b/src/MessageHandler/Database/EmailChangedNoticeEmailHandler.php index b9729288b..5c9e515ed 100644 --- a/src/MessageHandler/Database/EmailChangedNoticeEmailHandler.php +++ b/src/MessageHandler/Database/EmailChangedNoticeEmailHandler.php @@ -32,7 +32,7 @@ public function __invoke(EmailChangedNoticeEmail $message): void $message->getPreviousEmail(), $member->getFullName(), ), - 'Your e-mail address was changed (' . $member->lidnr . ')', + 'Your email address was changed (' . $member->lidnr . ')', 'database/email/email-changed.html.twig', [ 'firstName' => $member->firstName, diff --git a/src/MessageHandler/Database/GraduateRemovalRequestedHandler.php b/src/MessageHandler/Database/GraduateRemovalRequestedHandler.php new file mode 100644 index 000000000..1da0fb68c --- /dev/null +++ b/src/MessageHandler/Database/GraduateRemovalRequestedHandler.php @@ -0,0 +1,57 @@ +memberRepository->findSimple($message->getLidnr()); + + // Removed between the asking and now, which is the thing that was asked for. + if (null === $member) { + return; + } + + $this->emailService->send( + new Address( + $this->mailToSubscriptionAddress, + $this->mailToSubscriptionName, + ), + 'Removal requested by a leaving member (' . $member->lidnr . ')', + 'database/email/graduate-removal-requested.html.twig', + [ + 'member' => $member, + 'url' => $this->urlGenerator->generate( + 'member_show', + [ + '_locale' => Languages::English->getLangParam(), + 'lidnr' => $member->lidnr, + ], + UrlGeneratorInterface::ABSOLUTE_URL, + ), + ], + $this->emailService->secretary(), + ); + } +} diff --git a/src/Repository/Checker/MemberRepository.php b/src/Repository/Checker/MemberRepository.php index 35accb000..9906fb9f6 100644 --- a/src/Repository/Checker/MemberRepository.php +++ b/src/Repository/Checker/MemberRepository.php @@ -6,9 +6,11 @@ use App\Entity\Application\AssociationYear; use App\Entity\Database\Enums\MembershipTypes; +use App\Entity\Database\GraduateConversionLink; use App\Entity\Database\Member; use App\Entity\Database\Membership; use App\Entity\Database\RenewalLink; +use App\Repository\Database\SubDecision\FoundationRepository; use DateTimeImmutable; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; use Doctrine\ORM\QueryBuilder; @@ -82,6 +84,87 @@ public function getExpiringGraduates( return $qb->getQuery()->getResult(); } + /** + * An active member's membership is renewed by the board rather than ended, so they are not asked. + * + * @param ?DateTimeImmutable $expiresBefore Latest expiry date, end of current association year if null + * + * @return Member[] + */ + public function getExpiringConversions( + ?DateTimeImmutable $expiresBefore = null, + ?int $limit = null, + ): array { + $qb = $this->createQueryBuilder('m'); + + $qb->select('m, mem') + ->leftJoin( + 'm.memberships', + 'mem', + ) + ->where($qb->expr()->in( + 'mem.type', + ':convertible', + )) + ->andWhere('m.email IS NOT NULL') + ->andWhere('m.hidden = false') + ->andWhere('m.deleted = false') + ->andWhere($qb->expr()->eq('mem.startDate', '(' . $this->lastMembershipQuery()->getDQL() . ')')) + ->andWhere('mem.endDate <= :expiresBefore') + // Bounded at both ends, or the first run works through every member who ever left. + ->andWhere('mem.endDate >= :expiresAfter') + ->setParameter( + 'expiresAfter', + new DateTimeImmutable()->modify('-' . GraduateConversionLink::GRACE_DAYS . ' days')->setTime( + 0, + 0, + ), + ) + ->setParameter( + 'convertible', + [ + MembershipTypes::Ordinary, + MembershipTypes::External, + ], + ); + + $qbal = $this->getEntityManager()->createQueryBuilder(); + $qbal->select('gl') + ->from( + GraduateConversionLink::class, + 'gl', + ) + ->andWhere('gl.member = m') + ->andWhere('gl.currentExpiration = mem.endDate'); + + $qb->setParameter( + 'expiresBefore', + $expiresBefore ?? AssociationYear::fromDate(new DateTimeImmutable())->endsOn(), + ); + + $qb->andWhere($qb->expr()->not( + $qb->expr()->exists($qbal->getDQL()), + )); + + $today = new DateTimeImmutable(); + $active = FoundationRepository::getIsActiveWithinSubQuery( + qb: $qb, + activeAfter: $today, + activeBefore: $today, + ); + + $qb->andWhere($qb->expr()->notIn( + 'm', + $active->getDQL(), + )); + + if (null !== $limit) { + $qb->setMaxResults($limit); + } + + return $qb->getQuery()->getResult(); + } + /** * This helper query is used for multiple queries to get the LAST membership of a member. * This is not necessarily the current membership. diff --git a/src/Repository/Database/ActionLinkRepository.php b/src/Repository/Database/ActionLinkRepository.php index 37325d7f0..be8824adf 100644 --- a/src/Repository/Database/ActionLinkRepository.php +++ b/src/Repository/Database/ActionLinkRepository.php @@ -6,6 +6,7 @@ use App\Entity\Database\ActionLink; use App\Entity\Database\EmailChangeLink; +use App\Entity\Database\GraduateConversionLink; use App\Entity\Database\Member; use App\Entity\Database\PaymentLink; use App\Entity\Database\RenewalLink; @@ -120,6 +121,31 @@ public function findEmailChangeBySelector(string $selector): ?EmailChangeLink return $qb->getQuery()->getOneOrNullResult(); } + /** + * As {@see self::findPaymentBySelector()}, for the offer to stay on as a graduate. + */ + public function findGraduateConversionBySelector(string $selector): ?GraduateConversionLink + { + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->select('gl, m') + ->from( + GraduateConversionLink::class, + 'gl', + ) + ->leftJoin( + 'gl.member', + 'm', + ) + ->where('gl.selector = :selector'); + + $qb->setParameter( + 'selector', + $selector, + ); + + return $qb->getQuery()->getOneOrNullResult(); + } + public function findByTempHash(string $tempHash): ?ActionLink { return $this->findOneBy(['tempHash' => $tempHash]); diff --git a/src/Repository/Database/GraduateConversionLinkRepository.php b/src/Repository/Database/GraduateConversionLinkRepository.php new file mode 100644 index 000000000..c9c4d7a62 --- /dev/null +++ b/src/Repository/Database/GraduateConversionLinkRepository.php @@ -0,0 +1,65 @@ + + */ +class GraduateConversionLinkRepository extends ServiceEntityRepository +{ + public function __construct(ManagerRegistry $registry) + { + parent::__construct( + $registry, + GraduateConversionLink::class, + ); + } + + /** + * @return GraduateConversionLink[] + */ + public function findOutstandingForMember(Member $member): array + { + return $this->findBy([ + 'member' => $member, + 'used' => false, + ]); + } + + /** + * The members who have been asked and whose offer is still open, by membership number. + * + * @return int[] + */ + public function findMembersWithAnOpenOffer(): array + { + $rows = $this->createQueryBuilder('l') + ->select('IDENTITY(l.member) AS lidnr') + ->where('l.used = false') + ->andWhere('l.currentExpiration >= :stillOpen') + ->setParameter( + 'stillOpen', + new DateTimeImmutable()->modify('-' . GraduateConversionLink::GRACE_DAYS . ' days')->setTime( + 0, + 0, + ), + ) + ->getQuery() + ->getScalarResult(); + + return array_map( + static fn (array $row): int => (int) $row['lidnr'], + $rows, + ); + } +} diff --git a/src/Service/Checker/Renewal.php b/src/Service/Checker/Renewal.php index b5828ea20..1d469b5f8 100644 --- a/src/Service/Checker/Renewal.php +++ b/src/Service/Checker/Renewal.php @@ -5,6 +5,7 @@ namespace App\Service\Checker; use App\Entity\Application\Enums\Languages; +use App\Entity\Database\GraduateConversionLink as GraduateConversionLinkModel; use App\Entity\Database\RenewalLink as RenewalLinkModel; use App\Entity\Decision\OrganMember as OrganMemberModel; use App\Repository\Checker\MemberRepository; @@ -60,6 +61,71 @@ public function sendRenewalGraduates(): void } } + public function sendGraduateConversions(): void + { + $expiresWithin = new DateTimeImmutable()->add(new DateInterval('P45D')); + $limit = 10; + $members = $this->memberRepository->getExpiringConversions( + $expiresWithin, + $limit, + ); + + foreach ($members as $member) { + $membership = $member->getCurrentOrLastMembership(); + + if (null === $membership) { + continue; + } + + $link = new GraduateConversionLinkModel( + $member, + clone $membership->endDate, + ); + $this->actionLinkRepository->persist($link); + + try { + $this->sendGraduateConversionEmail($link); + } catch (Throwable $e) { + $this->actionLinkRepository->remove($link); + + throw $e; + } + } + } + + private function sendGraduateConversionEmail(GraduateConversionLinkModel $link): void + { + $member = $link->member; + $recipient = $member->getEmailRecipient(); + + if (null === $recipient) { + return; + } + + $this->emailService->send( + $recipient, + 'Your GEWIS membership is ending (' . $member->lidnr . ')', + 'database/email/graduate-conversion.html.twig', + [ + 'firstName' => $member->firstName, + 'currentExpiration' => $link->currentExpiration, + // English page, and the token this link was just minted with: only its hash is stored. + 'url' => $this->urlGenerator->generate( + 'join_graduate_claim', + [ + '_locale' => Languages::English->getLangParam(), + 'token' => $link->plainToken ?? throw new RuntimeException( + 'Cannot write a conversion link that was not minted here', + ), + ], + UrlGeneratorInterface::ABSOLUTE_URL, + ), + ], + $this->emailService->secretary(), + bccReplyTo: true, + ); + } + private function sendRenewalEmail(RenewalLinkModel $link): void { $reportMember = $this->reportMemberRepository->findSimple($link->member->lidnr); diff --git a/src/Service/Database/ActionLinkService.php b/src/Service/Database/ActionLinkService.php index eccd75cec..b367a7023 100644 --- a/src/Service/Database/ActionLinkService.php +++ b/src/Service/Database/ActionLinkService.php @@ -6,6 +6,7 @@ use App\Entity\Database\ActionLink; use App\Entity\Database\EmailChangeLink; +use App\Entity\Database\GraduateConversionLink; use App\Entity\Database\PaymentLink; use App\Entity\Database\RenewalLink; use App\Repository\Database\ActionLinkRepository; @@ -98,6 +99,39 @@ public function resolveEmailChange(string $token): ?EmailChangeLink return $link; } + public function resolveGraduateConversion(string $token): ?GraduateConversionLink + { + $split = SplitToken::split($token); + + if (null === $split) { + return null; + } + + $link = $this->actionLinkRepository->findGraduateConversionBySelector($split['selector']); + + if ( + null === $link + || $link->used + || $link->linkExpired() + || !$link->tokenMatches($split['verifier']) + ) { + return null; + } + + // The ending the offer was about has been settled since -- by the secretary's bulk conversion, or by the + // member somewhere else -- so following it would write the membership a second time. + $membership = $link->member->getCurrentOrLastMembership(); + + if ( + null === $membership + || $membership->endDate->getTimestamp() !== $link->currentExpiration->getTimestamp() + ) { + return null; + } + + return $link; + } + public function claim(ActionLink $link): string { $tempHash = bin2hex(random_bytes(32)); diff --git a/src/Service/Database/Member.php b/src/Service/Database/Member.php index fefc47ca3..b2542d451 100644 --- a/src/Service/Database/Member.php +++ b/src/Service/Database/Member.php @@ -13,6 +13,7 @@ use App\Entity\Database\EmailChangeLink as EmailChangeLinkModel; use App\Entity\Database\Enums\AddressTypes; use App\Entity\Database\Enums\AttentionReasons; +use App\Entity\Database\Enums\GraduateConversionOutcome; use App\Entity\Database\Enums\MailingListMemberAction; use App\Entity\Database\Enums\MailingListMemberOrigin; use App\Entity\Database\Enums\MemberDetailAction; @@ -20,6 +21,7 @@ use App\Entity\Database\Enums\PostalRegions; use App\Entity\Database\Enums\ProspectiveMemberFilter; use App\Entity\Database\Enums\Studies; +use App\Entity\Database\GraduateConversionLink as GraduateConversionLinkModel; use App\Entity\Database\MailingList as MailingListModel; use App\Entity\Database\MailingListMember as MailingListMemberModel; use App\Entity\Database\Member as MemberModel; @@ -29,12 +31,14 @@ use App\Entity\Database\RenewalLink as RenewalLinkModel; use App\Entity\User\User; use App\Form\Database\Registration\RegistrationData; +use App\Message\Database\GraduateRemovalRequested; use App\Message\Database\RefundProblemEmail; use App\Message\Database\RegistrationUpdate; use App\Message\Database\RegistrationUpdateEmail; use App\Repository\Database\ActionLinkRepository; use App\Repository\Database\AuditEntryRepository; use App\Repository\Database\EmailChangeLinkRepository; +use App\Repository\Database\GraduateConversionLinkRepository; use App\Repository\Database\MailingListMemberRepository; use App\Repository\Database\MailingListRepository; use App\Repository\Database\MemberRepository; @@ -69,6 +73,7 @@ public function __construct( private readonly AuditEntryRepository $auditEntryRepository, private readonly MemberRepository $memberRepository, private readonly EmailChangeLinkRepository $emailChangeLinkRepository, + private readonly GraduateConversionLinkRepository $graduateConversionLinkRepository, private readonly ProspectiveMemberRepository $prospectiveMemberRepository, private readonly MailingListService $mailingListService, private readonly RenewalService $renewalService, @@ -1193,10 +1198,25 @@ private function applyMembershipChange( $renewalAudit, ); $this->memberRepository->persist($member); + $this->supersedeGraduateConversions($member); return $member; } + /** + * An outstanding offer of graduate membership is about the ending that has just been settled, so following it + * would write the membership a second time. The secretary's bulk conversion goes through here, which is where + * the two meet. + */ + private function supersedeGraduateConversions(MemberModel $member): void + { + foreach ($this->graduateConversionLinkRepository->findOutstandingForMember($member) as $link) { + $link->outcome = GraduateConversionOutcome::Superseded; + $link->used = true; + $this->actionLinkRepository->persist($link); + } + } + private function persistAddressFromForm( FormInterface $form, MemberDetailAction $action, @@ -1277,6 +1297,7 @@ public function getMembersRequiringAttention(int $days = 30): array /** @var array, MemberModel[]> $combined */ $combined = []; + $askedThemselves = $this->graduateConversionLinkRepository->findMembersWithAnOpenOffer(); $combined[AttentionReasons::MissingEmail->value] = $this->memberRepository->findAttentionWithoutEmail(); $combined[AttentionReasons::MissingStudentNumberOrdinary->value] = @@ -1324,6 +1345,18 @@ public function getMembersRequiringAttention(int $days = 30): array if ($reason->includeBulkGraduateConversion()) { foreach ($combined[$reason->value] ?? [] as $member) { + // A member who has been asked themselves is left out while their offer is open. They come back + // once it expires unanswered, which is when the secretary is the one to settle it. + if ( + in_array( + $member->lidnr, + $askedThemselves, + true, + ) + ) { + continue; + } + $bulkRenewalShortcuts['expiring_non_active'][] = $member->lidnr; } } @@ -1402,6 +1435,47 @@ public function confirmEmailChange(EmailChangeLinkModel $link): MemberModel return $member; } + public function acceptGraduateConversion( + MemberModel $member, + GraduateConversionLinkModel $link, + ): MemberModel { + $link->outcome = GraduateConversionOutcome::Accepted; + $link->used = true; + $this->actionLinkRepository->persist($link); + + return $this->applyMembershipChange( + $member, + MembershipTypes::Graduate, + ); + } + + public function declineGraduateConversion( + MemberModel $member, + GraduateConversionLinkModel $link, + bool $removalRequested, + ): MemberModel { + $link->outcome = $removalRequested + ? GraduateConversionOutcome::RemovalRequested + : GraduateConversionOutcome::Declined; + $link->used = true; + $this->actionLinkRepository->persist($link); + + $note = new AuditNoteModel(); + $note->note = $removalRequested + ? 'Declined to stay on as a graduate and asked for their data to be removed.' + : 'Declined to stay on as a graduate.'; + $this->addAuditEntry( + $member, + $note, + ); + + if ($removalRequested) { + $this->bus->dispatch(new GraduateRemovalRequested($member->lidnr)); + } + + return $member; + } + /** * Whether this address already belongs to someone else (a member or an applicant). * diff --git a/src/Validator/Database/DeliverableEmailAddress.php b/src/Validator/Database/DeliverableEmailAddress.php index 8a5cf3d6f..1031367b3 100644 --- a/src/Validator/Database/DeliverableEmailAddress.php +++ b/src/Validator/Database/DeliverableEmailAddress.php @@ -20,7 +20,7 @@ class DeliverableEmailAddress extends Constraint protected const array ERROR_NAMES = [self::NO_MX_RECORD_ERROR => 'NO_MX_RECORD_ERROR']; // phpcs:ignore -- user-visible strings should not be split - public string $message = 'Please check your e-mail address, \'{{ hostname }}\' does not appear to be able to receive e-mails. If you are certain that your e-mail address is correct, please contact the board.'; + public string $message = 'Please check your email address, \'{{ hostname }}\' does not appear to be able to receive emails. If you are certain that your email address is correct, please contact the board.'; /** * @param string[]|null $groups diff --git a/src/Validator/Database/UnusedEmailAddress.php b/src/Validator/Database/UnusedEmailAddress.php index 1f775d0b1..b4e2a9a83 100644 --- a/src/Validator/Database/UnusedEmailAddress.php +++ b/src/Validator/Database/UnusedEmailAddress.php @@ -17,7 +17,7 @@ class UnusedEmailAddress extends Constraint protected const array ERROR_NAMES = [self::ALREADY_USED_ERROR => 'ALREADY_USED_ERROR']; - public string $message = 'There already is a member with this e-mail address.'; + public string $message = 'There already is a member with this email address.'; /** * @param string[]|null $groups diff --git a/templates/components/Database/MemberOverview.html.twig b/templates/components/Database/MemberOverview.html.twig index 93ceaca3a..18c04c9d6 100644 --- a/templates/components/Database/MemberOverview.html.twig +++ b/templates/components/Database/MemberOverview.html.twig @@ -4,7 +4,7 @@
    @@ -43,7 +43,7 @@ # {{ 'Name'|trans }} - {{ 'E-mail Address'|trans }} + {{ 'Email Address'|trans }} {{ 'Generation'|trans }} {{ 'Expiration'|trans }} {{ 'Status'|trans }} @@ -65,7 +65,7 @@ {% if member.email is empty %} - {{ 'No e-mail on record'|trans }} + {{ 'No email on record'|trans }} {% else %} {{ member.email }} {% endif %} diff --git a/templates/components/Database/ProspectiveMemberOverview.html.twig b/templates/components/Database/ProspectiveMemberOverview.html.twig index e03d6c516..c3ea8fdbf 100644 --- a/templates/components/Database/ProspectiveMemberOverview.html.twig +++ b/templates/components/Database/ProspectiveMemberOverview.html.twig @@ -4,7 +4,7 @@
    @@ -41,7 +41,7 @@ {{ 'Name'|trans }} - {{ 'E-mail Address'|trans }} + {{ 'Email Address'|trans }} {{ 'Registered'|trans }} {{ 'Checkout Status'|trans }} {{ 'Action'|trans }} diff --git a/templates/database/email/email-change-confirm.html.twig b/templates/database/email/email-change-confirm.html.twig index 77ce1ab1e..a7b535f7a 100644 --- a/templates/database/email/email-change-confirm.html.twig +++ b/templates/database/email/email-change-confirm.html.twig @@ -1,9 +1,9 @@ {% extends 'database/email/_base.html.twig' %} -{% block emailTitle %}Confirm your e-mail address{% endblock %} +{% block emailTitle %}Confirm your email address{% endblock %} {% block preheader %}{% endblock %} {% block heading %}Membership notification{% endblock %} -{% block subheading %}Confirm your e-mail address{% endblock %} +{% block subheading %}Confirm your email address{% endblock %} {% block body %}

    Dear {{ firstName }},

    @@ -21,4 +21,4 @@
    The Secretary of GEWIS

    {% endblock %} -{% block footerNote %}You receive this message because this address was entered as the new e-mail address of a GEWIS member. You can not opt-out of these emails.{% endblock %} +{% block footerNote %}You receive this message because this address was entered as the new email address of a GEWIS member. You can not opt-out of these emails.{% endblock %} diff --git a/templates/database/email/email-changed.html.twig b/templates/database/email/email-changed.html.twig index 5bb42fe8e..90ab2a2b3 100644 --- a/templates/database/email/email-changed.html.twig +++ b/templates/database/email/email-changed.html.twig @@ -1,13 +1,13 @@ {% extends 'database/email/_base.html.twig' %} -{% block emailTitle %}Your e-mail address was changed{% endblock %} +{% block emailTitle %}Your email address was changed{% endblock %} {% block preheader %}{% endblock %} {% block heading %}Membership notification{% endblock %} -{% block subheading %}Your e-mail address was changed{% endblock %} +{% block subheading %}Your email address was changed{% endblock %} {% block body %}

    Dear {{ firstName }},

    - The e-mail address of your GEWIS membership was changed from {{ previousEmail }} to + The email address of your GEWIS membership was changed from {{ previousEmail }} to {{ newEmail }}. From now on we write to the new address, and you sign in with it as well.

    @@ -19,4 +19,4 @@
    The Secretary of GEWIS

    {% endblock %} -{% block footerNote %}You receive this message because the e-mail address of your GEWIS membership was changed. You can not opt-out of these emails.{% endblock %} +{% block footerNote %}You receive this message because the email address of your GEWIS membership was changed. You can not opt-out of these emails.{% endblock %} diff --git a/templates/database/email/graduate-conversion.html.twig b/templates/database/email/graduate-conversion.html.twig new file mode 100644 index 000000000..17fc49f75 --- /dev/null +++ b/templates/database/email/graduate-conversion.html.twig @@ -0,0 +1,30 @@ +{% extends 'database/email/_base.html.twig' %} + +{% block emailTitle %}Your GEWIS membership is ending{% endblock %} +{% block preheader %}{% endblock %} +{% block heading %}Membership notification{% endblock %} +{% block subheading %}Staying on as a graduate{% endblock %} +{% block body %} +

    Dear {{ firstName }},

    +

    + Your GEWIS membership ends on {{ currentExpiration|date('F jS, Y') }}. If you have finished studying, + you can stay with the association as a graduate (Afgestudeerde): you keep receiving our announcements + and remain welcome at our activities. +

    +

    + Please click + here + to tell us what you would like to do. You can check the address we write to while you are there, because the + one we have may be your TU/e address, which stops working once you leave. +

    +

    + If you do nothing, your membership simply ends on the date above and we delete your data in the way described + in our + privacy statement. +

    +

    + Kind regards, +
    The Secretary of GEWIS +

    +{% endblock %} +{% block footerNote %}You receive this message because your GEWIS membership is about to end. You can not opt-out of these emails.{% endblock %} diff --git a/templates/database/email/graduate-removal-requested.html.twig b/templates/database/email/graduate-removal-requested.html.twig new file mode 100644 index 000000000..7b98a5460 --- /dev/null +++ b/templates/database/email/graduate-removal-requested.html.twig @@ -0,0 +1,22 @@ +{% extends 'database/email/_base.html.twig' %} + +{% block emailTitle %}Removal requested{% endblock %} +{% block preheader %}{% endblock %} +{% block heading %}Membership notification{% endblock %} +{% block subheading %}A leaving member asked to be removed{% endblock %} +{% block body %} +

    Dear secretary,

    +

    + {{ member.getFullName }} ({{ member.lidnr }}) has decided not to stay on as a graduate, and asked + for their data to be removed. +

    +

    + Their membership runs out on its own; nothing has been removed. The member's record is + here. +

    +

    + Kind regards, +
    The GEWIS website +

    +{% endblock %} +{% block footerNote %}You receive this message because you keep the register.{% endblock %} diff --git a/templates/database/email/member-registration.html.twig b/templates/database/email/member-registration.html.twig index 7eff0981c..e9556d0e7 100644 --- a/templates/database/email/member-registration.html.twig +++ b/templates/database/email/member-registration.html.twig @@ -19,7 +19,7 @@ {label: 'Last name', value: member.lastName}, {label: 'TU/e student number', value: member.studentNumber ?? 'Unknown'}, {label: 'Study', value: member.study.getName|trans}, - {label: 'E-mail address', value: member.email, href: 'mailto:' ~ member.email}, + {label: 'Email address', value: member.email, href: 'mailto:' ~ member.email}, {label: 'Birthdate', value: member.birth|date('l j F Y')}, ]) }} {{ details.summary('Address Information', [ diff --git a/templates/database/join/checkout-status.html.twig b/templates/database/join/checkout-status.html.twig index 941cc893f..800bda37e 100644 --- a/templates/database/join/checkout-status.html.twig +++ b/templates/database/join/checkout-status.html.twig @@ -14,12 +14,12 @@ {% elseif 'completed' == status %} @@ -29,12 +29,12 @@ ? 'Awesome!'|trans : 'Thank you %name%!'|trans({'%name%': first_name}) }} - {{ 'We have received your registration, once your payment is verified the secretary will approve your membership. We will let you know via e-mail when this happens.'|trans }} + {{ 'We have received your registration, once your payment is verified the secretary will approve your membership. We will let you know via email when this happens.'|trans }} {% else %} {% endif %} diff --git a/templates/database/join/graduate-declined.html.twig b/templates/database/join/graduate-declined.html.twig new file mode 100644 index 000000000..35d570d8d --- /dev/null +++ b/templates/database/join/graduate-declined.html.twig @@ -0,0 +1,25 @@ +{% extends 'layout.html.twig' %} + + +{% block page_title %}{{ 'Staying on as a graduate'|trans|page_title }}{{ parent() }}{% endblock %} + +{% block contents %} + {% embed 'database/partials/join/registration-header.html.twig' with {header_title: 'Your membership will end'|trans} %} + {% block registration_intro %} +

    + {{ 'Thank you for letting us know, %name%. Your membership ends on %date% and we will not ask you about it again.'|trans({ + '%name%': member.firstName, + '%date%': expiration|format_date(pattern: 'd MMMM y'), + }) }} +

    + {% if removal_requested %} +

    + {{ 'We have passed your request to have your data removed on to the secretary, who will be in touch.'|trans }} +

    + {% endif %} +

    + {{ 'Thank you for the years you were with us.'|trans }} +

    + {% endblock %} + {% endembed %} +{% endblock %} diff --git a/templates/database/join/graduate-done.html.twig b/templates/database/join/graduate-done.html.twig new file mode 100644 index 000000000..0e1a69401 --- /dev/null +++ b/templates/database/join/graduate-done.html.twig @@ -0,0 +1,16 @@ +{% extends 'layout.html.twig' %} + + +{% block page_title %}{{ 'Staying on as a graduate'|trans|page_title }}{{ parent() }}{% endblock %} + +{% block contents %} + {% embed 'database/partials/join/registration-header.html.twig' with {header_title: 'You are staying on as a graduate'|trans} %} + {% block registration_intro %} +

    + {{ 'Thank you %name%! Your membership continues as a graduate membership.'|trans({ + '%name%': member.firstName, + }) }} +

    + {% endblock %} + {% endembed %} +{% endblock %} diff --git a/templates/database/join/graduate-unavailable.html.twig b/templates/database/join/graduate-unavailable.html.twig new file mode 100644 index 000000000..fd29657ac --- /dev/null +++ b/templates/database/join/graduate-unavailable.html.twig @@ -0,0 +1,17 @@ +{% extends 'layout.html.twig' %} + + +{% block page_title %}{{ 'Staying on as a graduate'|trans|page_title }}{{ parent() }}{% endblock %} + +{% block contents %} + {% embed 'database/partials/join/registration-header.html.twig' with {header_title: 'This link no longer works'|trans} %} + {% block registration_intro %} +

    + {{ 'It was either used already or it has expired.'|trans }} +

    +

    + {{ 'If you would still like to stay on as a graduate, let the secretary know.'|trans }} +

    + {% endblock %} + {% endembed %} +{% endblock %} diff --git a/templates/database/join/graduate.html.twig b/templates/database/join/graduate.html.twig new file mode 100644 index 000000000..297705d9b --- /dev/null +++ b/templates/database/join/graduate.html.twig @@ -0,0 +1,81 @@ +{% extends 'layout.html.twig' %} + + +{% block page_title %}{{ 'Staying on as a graduate'|trans|page_title }}{{ parent() }}{% endblock %} + +{% block contents %} + {% embed 'database/partials/join/registration-header.html.twig' with {header_title: 'Your membership is ending'|trans} %} + {% block registration_intro %} +

    + {{ 'Your GEWIS membership ends on %date%. If you have finished studying, you can stay with the association as a graduate.'|trans({ + '%date%': expiration|format_date(pattern: 'd MMMM y'), + }) }} +

    +

    + {{ 'Check the address we write to before you confirm: if it is your TU/e address, it stops working once you leave.'|trans }} +

    +

    + {{ 'Doing nothing is also an answer: your membership simply ends on the date above.'|trans }} +

    + {% endblock %} + {% endembed %} + + {{ form_start(form, {attr: {id: 'form-graduate'}}) }} + + {% if form.vars.submitted and not form.vars.valid %} + + {% endif %} + +
    +

    {{ 'Personal details'|trans }}

    +
    +
    +
    + {{ form_row(form.initials) }} +
    +
    + {{ form_row(form.firstName) }} +
    +
    + {{ form_row(form.middleName) }} +
    +
    + {{ form_row(form.lastName) }} +
    +
    + {{ form_row(form.email) }} +
    +
    + +
    +

    {{ 'Supremum'|trans }}

    +
    +

    + {{ 'As a graduate, you can choose to keep receiving the physical Supremum magazine.'|trans }} + {{ 'You can always read the online Supremum on %link%.'|trans({ + '%link%': 'supremum.gewis.nl', + })|raw }} +

    + {{ form_row(form.supremum) }} +
    +
    + +
    +

    {{ 'If you would rather leave'|trans }}

    +
    +

    + {{ 'Tell us and we stop asking. Your membership ends on the date above either way.'|trans }} +

    + {{ form_row(form.removal) }} + {{ form_widget(form.decline, {attr: {class: 'btn-outline-danger'}}) }} +
    +
    + +
    + {{ form_widget(form.accept, {attr: {class: 'btn-gewis-primary'}}) }} +
    + + {{ form_end(form) }} +{% endblock %} diff --git a/templates/database/join/prospective-member/show.html.twig b/templates/database/join/prospective-member/show.html.twig index 59d0fe781..8254a3f4d 100644 --- a/templates/database/join/prospective-member/show.html.twig +++ b/templates/database/join/prospective-member/show.html.twig @@ -70,7 +70,7 @@ {{ member.studentNumber ?? 'Unknown'|trans }} - {{ 'E-mail Address'|trans }} + {{ 'Email Address'|trans }} {{ member.email }} diff --git a/templates/database/join/renew-done.html.twig b/templates/database/join/renew-done.html.twig index 74a307047..005c016f5 100644 --- a/templates/database/join/renew-done.html.twig +++ b/templates/database/join/renew-done.html.twig @@ -7,7 +7,7 @@ {% embed 'database/partials/join/registration-header.html.twig' with {header_title: 'Your graduate status has been renewed'|trans} %} {% block registration_intro %}

    - {{ 'Thank you %name%! We have sent you a confirmation by e-mail.'|trans({ + {{ 'Thank you %name%! We have sent you a confirmation by email.'|trans({ '%name%': member.firstName, }) }}

    diff --git a/templates/database/join/subscribe.html.twig b/templates/database/join/subscribe.html.twig index ff6d90d5a..72f5214ce 100644 --- a/templates/database/join/subscribe.html.twig +++ b/templates/database/join/subscribe.html.twig @@ -133,7 +133,7 @@
    {{ review.review_row('Name'|trans, data.fullName, 'personal', form) }} - {{ review.review_row('E-mail Address'|trans, data.email, 'personal', form) }} + {{ review.review_row('Email Address'|trans, data.email, 'personal', form) }} {{ review.review_row('Birthdate'|trans, data.birth ? data.birth|date('Y-m-d') : null, 'personal', form) }} {{ review.review_row('TU/e student number'|trans, data.studentNumber, 'study', form) }} {{ review.review_row('Study'|trans, data.study ? data.study.value|trans : null, 'study', form) }} diff --git a/templates/database/member/attention-needed.html.twig b/templates/database/member/attention-needed.html.twig index b18b60d81..e2f5fd437 100644 --- a/templates/database/member/attention-needed.html.twig +++ b/templates/database/member/attention-needed.html.twig @@ -49,6 +49,9 @@

    {{ 'Each shortcut opens the bulk renewal form with these members already filled in. Renew the members who are still studying first.'|trans|raw }}

    +

    + {{ 'Members who have been asked themselves whether they want to stay on as a graduate are left out until their own offer expires unanswered.'|trans }} +

    {{ bulk.shortcut( bulk_renewal_shortcuts.expiring_active, diff --git a/templates/database/member/show.html.twig b/templates/database/member/show.html.twig index c55d14edc..260776064 100644 --- a/templates/database/member/show.html.twig +++ b/templates/database/member/show.html.twig @@ -77,7 +77,7 @@ {{ member.studentNumber ?? 'Unknown'|trans }} - {{ 'E-mail Address'|trans }} + {{ 'Email Address'|trans }} {% if null is not same as(member.email) %} {{ member.email }} diff --git a/templates/user/email-change.html.twig b/templates/user/email-change.html.twig index 2c3f20f76..2d0a5852c 100644 --- a/templates/user/email-change.html.twig +++ b/templates/user/email-change.html.twig @@ -1,13 +1,13 @@ {% extends 'layout.html.twig' %} -{% block page_title %}{{ 'Confirm your e-mail address'|trans }}{% endblock %} +{% block page_title %}{{ 'Confirm your email address'|trans }}{% endblock %} {% block contents %}
    -

    {{ 'Confirm your e-mail address'|trans }}

    +

    {{ 'Confirm your email address'|trans }}

    {% if link is null %} diff --git a/templates/user/settings/details.html.twig b/templates/user/settings/details.html.twig index 1bb515e95..e94943344 100644 --- a/templates/user/settings/details.html.twig +++ b/templates/user/settings/details.html.twig @@ -16,7 +16,7 @@
    -

    {{ 'E-mail address'|trans }}

    +

    {{ 'Email address'|trans }}

    @@ -109,7 +109,7 @@

    -

    {{ 'What the secretary keeps'|trans }}

    +

    {{ 'Membership details'|trans }}

    @@ -133,7 +133,7 @@

    - {{ 'These are recorded by the secretary. Write to them if any of it is wrong.'|trans }} + {{ 'The secretary keeps these. Write to them if any of it is wrong.'|trans }}

    diff --git a/tests/Integration/Controller/Database/GraduateConversionTest.php b/tests/Integration/Controller/Database/GraduateConversionTest.php new file mode 100644 index 000000000..ddedd7906 --- /dev/null +++ b/tests/Integration/Controller/Database/GraduateConversionTest.php @@ -0,0 +1,313 @@ +get('doctrine')->getManager('default'); + self::assertInstanceOf( + EntityManagerInterface::class, + $ledger, + ); + + $this->ledger = $ledger; + } + + public function testAcceptingWritesAGraduateMembership(): void + { + $member = $this->expiringMember(); + $link = $this->link($member); + $controller = self::getContainer()->get(ProspectiveMemberController::class); + + $request = $this->pushRequest(); + $this->carryTempHash( + $request, + $controller->graduateClaim((string) $link->plainToken), + ); + $controller->graduate($request); + $request->query->remove('th'); + + $submission = $this->submission( + $member, + 'accept', + ); + $this->pushRequest($submission); + + $response = $controller->graduate($submission); + + self::assertSame( + Response::HTTP_OK, + $response->getStatusCode(), + ); + self::assertSame( + MembershipTypes::Graduate, + $member->getCurrentOrLastMembership()?->type, + ); + self::assertSame( + GraduateConversionOutcome::Accepted, + $link->outcome, + ); + self::assertTrue($link->used); + } + + public function testDecliningRecordsTheAnswerAndLeavesTheMembershipAlone(): void + { + $member = $this->expiringMember(); + $before = $member->getCurrentOrLastMembership()?->type; + $link = $this->link($member); + $controller = self::getContainer()->get(ProspectiveMemberController::class); + + $request = $this->pushRequest(); + $this->carryTempHash( + $request, + $controller->graduateClaim((string) $link->plainToken), + ); + $controller->graduate($request); + + $submission = $this->submission( + $member, + 'decline', + ); + $this->pushRequest($submission); + + $controller->graduate($submission); + + self::assertSame( + GraduateConversionOutcome::Declined, + $link->outcome, + ); + self::assertSame( + $before, + $member->getCurrentOrLastMembership()?->type, + ); + } + + public function testTheSecretarySettlingTheEndingRetiresTheOffer(): void + { + $member = $this->expiringMember(); + $link = $this->link($member); + + self::getContainer()->get(MemberService::class)->bulkRenewal( + (string) $member->lidnr, + MembershipTypes::Graduate, + true, + ); + + self::assertSame( + GraduateConversionOutcome::Superseded, + $link->outcome, + ); + self::assertTrue($link->used); + self::assertNull( + self::getContainer()->get(ActionLinkService::class)->resolveGraduateConversion( + (string) $link->plainToken, + ), + 'A link whose ending has been settled writes no second membership.', + ); + } + + /** + * The batch is for the members the sweep cannot settle: whoever it has asked keeps their window. + */ + public function testTheBulkShortcutLeavesOutAMemberWhoWasAsked(): void + { + $service = self::getContainer()->get(MemberService::class); + $before = $service->getMembersRequiringAttention()['bulk_renewal_shortcuts']['expiring_non_active']; + + self::assertNotEmpty( + $before, + 'The seed is expected to contain an expiring member the secretary could convert.', + ); + + $member = $this->ledger->getRepository(Member::class)->find($before[0]); + self::assertInstanceOf( + Member::class, + $member, + ); + + $this->link($member); + + $after = $service->getMembersRequiringAttention()['bulk_renewal_shortcuts']['expiring_non_active']; + + self::assertNotContains( + $member->lidnr, + $after, + ); + self::assertSame( + [$member->lidnr], + array_values(array_diff($before, $after)), + 'Only the member who was asked drops out of the batch.', + ); + } + + public function testALinkThatWasAnsweredIsRefused(): void + { + $member = $this->expiringMember(); + $link = $this->link($member); + $token = (string) $link->plainToken; + $link->used = true; + $this->ledger->flush(); + + $this->pushRequest(); + + $response = self::getContainer()->get(ProspectiveMemberController::class)->graduateClaim($token); + + self::assertSame( + Response::HTTP_OK, + $response->getStatusCode(), + ); + self::assertStringContainsString( + 'no longer works', + (string) $response->getContent(), + ); + } + + /** + * @return Request the submission, with the fields the form binds to the member + */ + private function submission( + Member $member, + string $button, + ): Request { + $request = new Request( + request: [ + 'member_graduate_conversion' => [ + 'email' => (string) $member->email, + 'supremum' => '1', + $button => '', + '_csrf_token' => 'csrf-token', + ], + ], + server: ['HTTP_SEC_FETCH_SITE' => 'same-origin'], + ); + $request->setMethod(Request::METHOD_POST); + + return $request; + } + + private function carryTempHash( + Request $request, + Response $claim, + ): void { + self::assertInstanceOf( + RedirectResponse::class, + $claim, + ); + + $query = []; + parse_str( + (string) parse_url( + $claim->getTargetUrl(), + PHP_URL_QUERY, + ), + $query, + ); + + $request->query->set( + 'th', + $query['th'] ?? '', + ); + } + + private function link(Member $member): GraduateConversionLink + { + $membership = $member->getCurrentOrLastMembership(); + self::assertNotNull($membership); + + $link = new GraduateConversionLink( + $member, + DateTimeImmutable::createFromInterface($membership->endDate), + ); + $this->ledger->persist($link); + $this->ledger->flush(); + + return $link; + } + + private function expiringMember(): Member + { + $member = $this->ledger->getRepository(Member::class)->createQueryBuilder('m') + ->join( + 'm.memberships', + 'mem', + ) + ->where('mem.type = :ordinary') + ->andWhere('m.email IS NOT NULL') + ->andWhere('m.deleted = false') + ->setParameter( + 'ordinary', + MembershipTypes::Ordinary, + ) + ->setMaxResults(1) + ->getQuery() + ->getOneOrNullResult(); + + self::assertInstanceOf( + Member::class, + $member, + 'The seed is expected to contain an ordinary member.', + ); + + return $member; + } + + /** + * Shared by both stages: the first binds the link to it, the second reads it back. + */ + private ?SessionInterface $session = null; + + private function pushRequest(?Request $request = null): Request + { + if (null === $this->session) { + $session = self::getContainer()->get('session.factory')->createSession(); + self::assertInstanceOf( + FlashBagAwareSessionInterface::class, + $session, + ); + + $this->session = $session; + } + + $request ??= new Request(); + $request->setLocale('en'); + $request->setSession($this->session); + self::getContainer()->get('request_stack')->push($request); + + return $request; + } +} diff --git a/tests/Integration/MessageHandler/Database/GraduateRemovalRequestedHandlerTest.php b/tests/Integration/MessageHandler/Database/GraduateRemovalRequestedHandlerTest.php new file mode 100644 index 000000000..6e6e51310 --- /dev/null +++ b/tests/Integration/MessageHandler/Database/GraduateRemovalRequestedHandlerTest.php @@ -0,0 +1,94 @@ +get('doctrine')->getManager('default'); + self::assertInstanceOf( + EntityManagerInterface::class, + $ledger, + ); + + $this->ledger = $ledger; + } + + public function testReachesTheSecretaryAndSaysWhoAsked(): void + { + $member = $this->member(); + + self::getContainer()->get(GraduateRemovalRequestedHandler::class)->__invoke( + new GraduateRemovalRequested($member->lidnr), + ); + + self::assertEmailCount(1); + $email = self::getMailerMessage(); + self::assertInstanceOf( + Email::class, + $email, + ); + + $body = (string) $email->getHtmlBody(); + + self::assertStringContainsString( + $member->getFullName(), + $body, + ); + self::assertStringContainsString( + (string) $member->lidnr, + $body, + ); + // The record itself, so the secretary can act on it without going looking. + self::assertStringContainsString( + '/admin/members/' . $member->lidnr, + $body, + ); + } + + public function testSaysNothingAboutAMemberWhoIsAlreadyGone(): void + { + self::getContainer()->get(GraduateRemovalRequestedHandler::class)->__invoke( + new GraduateRemovalRequested(999999), + ); + + self::assertEmailCount(0); + } + + private function member(): Member + { + $member = $this->ledger->getRepository(Member::class)->createQueryBuilder('m') + ->where('m.deleted = false') + ->setMaxResults(1) + ->getQuery() + ->getOneOrNullResult(); + + self::assertInstanceOf( + Member::class, + $member, + ); + + return $member; + } +} diff --git a/tests/Integration/Service/Checker/GraduateConversionSweepTest.php b/tests/Integration/Service/Checker/GraduateConversionSweepTest.php new file mode 100644 index 000000000..82ea80a91 --- /dev/null +++ b/tests/Integration/Service/Checker/GraduateConversionSweepTest.php @@ -0,0 +1,140 @@ +get('doctrine')->getManager('default'); + self::assertInstanceOf( + EntityManagerInterface::class, + $ledger, + ); + + $this->ledger = $ledger; + } + + public function testWritesToAMemberWhoseMembershipIsAboutToRunOut(): void + { + self::getContainer()->get(RenewalService::class)->sendGraduateConversions(); + + $link = $this->ledger->getRepository(GraduateConversionLink::class)->findOneBy([]); + self::assertInstanceOf( + GraduateConversionLink::class, + $link, + 'The seed is expected to contain a member whose membership is about to run out.', + ); + + $member = $link->member; + $email = $this->emailTo((string) $member->email); + + self::assertNotNull( + $email, + 'The member the offer was recorded for is the member it was sent to.', + ); + + $body = (string) $email->getHtmlBody(); + + // The link is the whole of it: without one that resolves, the message is an announcement nobody can act on. + self::assertSame( + 1, + preg_match( + '{/en/graduate/([0-9a-f]{32}\.[0-9a-f]{96})}', + $body, + $matches, + ), + ); + self::assertSame( + $link->id, + self::getContainer()->get(ActionLinkService::class)->resolveGraduateConversion($matches[1])?->id, + ); + } + + public function testDoesNotAskAboutTheSameEndingTwice(): void + { + $service = self::getContainer()->get(RenewalService::class); + $service->sendGraduateConversions(); + + $after = $this->ledger->getRepository(GraduateConversionLink::class)->count([]); + self::assertGreaterThan( + 0, + $after, + ); + + $service->sendGraduateConversions(); + + self::assertSame( + $after, + $this->ledger->getRepository(GraduateConversionLink::class)->count([]), + ); + } + + public function testLeavesAMembershipThatEndedLongAgoAlone(): void + { + $found = self::getContainer()->get(CheckerMemberRepository::class)->getExpiringConversions(); + + foreach ($found as $member) { + $membership = $member->getCurrentOrLastMembership(); + self::assertNotNull($membership); + + self::assertGreaterThanOrEqual( + new DateTimeImmutable()->modify('-' . GraduateConversionLink::GRACE_DAYS . ' days')->setTime( + 0, + 0, + ), + $membership->endDate, + ); + self::assertContains( + $membership->type, + [ + MembershipTypes::Ordinary, + MembershipTypes::External, + ], + ); + } + } + + private function emailTo(string $address): ?Email + { + foreach (self::getMailerMessages() as $message) { + if (!$message instanceof Email) { + continue; + } + + foreach ($message->getTo() as $to) { + if ($to->getAddress() === $address) { + return $message; + } + } + } + + return null; + } +} diff --git a/tests/Scheduler/MainScheduleTest.php b/tests/Scheduler/MainScheduleTest.php index fdd5698cf..2647d8b4f 100644 --- a/tests/Scheduler/MainScheduleTest.php +++ b/tests/Scheduler/MainScheduleTest.php @@ -58,6 +58,7 @@ class MainScheduleTest extends KernelTestCase 'app:user:update-ip-databases' => 'maintenance', 'app:users:force-relogin' => 'cron', 'check:database' => 'maintenance', + 'check:membership:conversion:graduate' => 'maintenance', 'check:membership:renewal:graduate' => 'maintenance', 'database:mailinglist:fetch all' => 'maintenance', 'database:mailinglist:maintenance -f -vv' => 'maintenance', diff --git a/translations/messages.en.xlf b/translations/messages.en.xlf index 6919f3a02..c54a46dfa 100644 --- a/translations/messages.en.xlf +++ b/translations/messages.en.xlf @@ -841,6 +841,10 @@ Accept your invitation Accept your invitation + + Accepted + Accepted + Access Denied Access Denied @@ -1349,6 +1353,10 @@ Already in the agenda Already in the agenda + + Also ask the secretary to remove my data + Also ask the secretary to remove my data + Also in another list Also in another list @@ -1381,9 +1389,9 @@ An error occurred while we were trying to prepare the checkout page. Please try again later. An error occurred while we were trying to prepare the checkout page. Please try again later. - - An error occurred while we were trying to prepare the checkout page. We have received your registration and sent you an e-mail containing a link that allows you to restart the checkout process at a later time. - An error occurred while we were trying to prepare the checkout page. We have received your registration and sent you an e-mail containing a link that allows you to restart the checkout process at a later time. + + An error occurred while we were trying to prepare the checkout page. We have received your registration and sent you an email containing a link that allows you to restart the checkout process at a later time. + An error occurred while we were trying to prepare the checkout page. We have received your registration and sent you an email containing a link that allows you to restart the checkout process at a later time. An image of this page @@ -2129,6 +2137,10 @@ Change Subscriptions Change Subscriptions + + Change email address + Change email address + Change it Change it @@ -2165,6 +2177,10 @@ Changing this asks the company for artwork at the new size; the banner it already has stays up until it hands one over. Changing this asks the company for artwork at the new size; the banner it already has stays up until it hands one over. + + Check the address we write to before you confirm: if it is your TU/e address, it stops working once you leave. + Check the address we write to before you confirm: if it is your TU/e address, it stops working once you leave. + Checkout Status Checkout Status @@ -2477,6 +2493,10 @@ Confirm sign-up Confirm sign-up + + Confirm your email address + Confirm your email address + Confirm your identity Confirm your identity @@ -2941,6 +2961,10 @@ Declined Declined + + Declined, removal requested + Declined, removal requested + Default Default @@ -3213,6 +3237,10 @@ Doing nothing is also an answer: your graduate status simply expires on the date shown below. Doing nothing is also an answer: your graduate status simply expires on the date shown below. + + Doing nothing is also an answer: your membership simply ends on the date above. + Doing nothing is also an answer: your membership simply ends on the date above. + Done Done @@ -3305,10 +3333,6 @@ Dutch Description Dutch Description - - E-mail Address - E-mail Address - ES512 ES512 @@ -3441,6 +3465,10 @@ Email %count% selected Email %count% selected + + Email Address + Email Address + Email address Email address @@ -3517,6 +3545,10 @@ End date and time End date and time + + End my membership + End my membership + End sessions End sessions @@ -4669,10 +4701,18 @@ If you hide yours, you also stop seeing others' age. The board can always see it. If you hide yours, you also stop seeing others' age. The board can always see it. + + If you would rather leave + If you would rather leave + If you would rather remember it, take several random words and add characters that are not words. Take the words at random, not a phrase about yourself. %example% has leaked, so pick your own. See %link%. If you would rather remember it, take several random words and add characters that are not words. Take the words at random, not a phrase about yourself. %example% has leaked, so pick your own. See %link%. + + If you would still like to stay on as a graduate, let the secretary know. + If you would still like to stay on as a graduate, let the secretary know. + If your browser does not redirect you, %link% manually. If your browser does not redirect you, %link% manually. @@ -5373,9 +5413,9 @@ Mailing lists - List mailing lists Mailing lists - List mailing lists - - Mailing lists - List subscribers of a mailing list, including their e-mail addresses - Mailing lists - List subscribers of a mailing list, including their e-mail addresses + + Mailing lists - List subscribers of a mailing list, including their email addresses + Mailing lists - List subscribers of a mailing list, including their email addresses Mailman Mailing List @@ -5617,6 +5657,10 @@ Members who are still studying become ordinary. Members who are still studying become ordinary. + + Members who have been asked themselves whether they want to stay on as a graduate are left out until their own offer expires unanswered. + Members who have been asked themselves whether they want to stay on as a graduate are left out until their own offer expires unanswered. + Members' accounts Members' accounts @@ -5637,6 +5681,10 @@ Membership Type (today) Membership Type (today) + + Membership details + Membership details + Membership number Membership number @@ -5917,13 +5965,13 @@ Name this query Name this query - - Name, e-mail address or student number - Name, e-mail address or student number + + Name, email address or student number + Name, email address or student number - - Name, e-mail address, member number or student number - Name, e-mail address, member number or student number + + Name, email address, member number or student number + Name, email address, member number or student number Navigation @@ -5977,6 +6025,10 @@ New backup codes were generated for your account from %name%. New backup codes were generated for your account from %name%. + + New email address + New email address + New expiration New expiration @@ -6165,14 +6217,14 @@ No documents were uploaded under this agenda point. No documents were uploaded under this agenda point. - - No e-mail on record - No e-mail on record - No email address No email address + + No email on record + No email on record + No end time; meetings run until closed. No end time; meetings run until closed. @@ -6441,6 +6493,10 @@ Not activated Not activated + + Not answered + Not answered + Not answered by me Not answered by me @@ -8761,6 +8817,10 @@ Settings Settings + + Settled by the secretary + Settled by the secretary + Shared by a member Shared by a member @@ -9149,10 +9209,18 @@ Status Status + + Stay on as a graduate + Stay on as a graduate + Staying at the university Staying at the university + + Staying on as a graduate + Staying on as a graduate + Stemcommissie Voting Committee @@ -9373,6 +9441,10 @@ Taking proposals Taking proposals + + Tell us and we stop asking. Your membership ends on the date above either way. + Tell us and we stop asking. Your membership ends on the date above either way. + Terminate Terminate @@ -9405,9 +9477,21 @@ Thank you %name%! Thank you %name%! - - Thank you %name%! We have sent you a confirmation by e-mail. - Thank you %name%! We have sent you a confirmation by e-mail. + + Thank you %name%! We have sent you a confirmation by email. + Thank you %name%! We have sent you a confirmation by email. + + + Thank you %name%! Your membership continues as a graduate membership. + Thank you %name%! Your membership continues as a graduate membership. + + + Thank you for letting us know, %name%. Your membership ends on %date% and we will not ask you about it again. + Thank you for letting us know, %name%. Your membership ends on %date% and we will not ask you about it again. + + + Thank you for the years you were with us. + Thank you for the years you were with us. That action is not available for this revision. @@ -10005,6 +10089,10 @@ The round is open. Every body may propose without anything being set up for it. The round is open. Every body may propose without anything being set up for it. + + The secretary keeps these. Write to them if any of it is wrong. + The secretary keeps these. Write to them if any of it is wrong. + The selected photos are already in that album. The selected photos are already in that album. @@ -10085,9 +10173,9 @@ Theme Theme - - There already is a member with this e-mail address. - There already is a member with this e-mail address. + + There already is a member with this email address. + There already is a member with this email address. There are currently no committees installed. @@ -10405,6 +10493,10 @@ This is %source%. This is %source%. + + This is already your email address. + This is already your email address. + This is the address the association writes to, and the one you sign in with. This is the address the association writes to, and the one you sign in with. @@ -11025,6 +11117,10 @@ Use %number% Use %number% + + Use an address you keep after you stop studying. + Use an address you keep after you stop studying. + Use “quotes” for exact phrases, -word to exclude, type:bm to limit to one meeting type, and meeting:1805 to limit to one meeting. Use “quotes” for exact phrases, -word to exclude, type:bm to limit to one meeting type, and meeting:1805 to limit to one meeting. @@ -11301,13 +11397,13 @@ Was Was - - Was it not your intention to close the payment page? Then do not worry, <a href="%url%">click here</a> to go back. Are you unable to finish the payment now? Then also do not worry, we have sent you an e-mail containing a link that allows you to go to the payment page again. Do you not want to become a member of GEWIS after all? Let us know and we will delete your details. - click here to go back. Are you unable to finish the payment now? Then also do not worry, we have sent you an e-mail containing a link that allows you to go to the payment page again. Do you not want to become a member of GEWIS after all? Let us know and we will delete your details.]]> + + Was it not your intention to close the payment page? Then do not worry, <a href="%url%">click here</a> to go back. Are you unable to finish the payment now? Then also do not worry, we have sent you an email containing a link that allows you to go to the payment page again. Do you not want to become a member of GEWIS after all? Let us know and we will delete your details. + Was it not your intention to close the payment page? Then do not worry, <a href="%url%">click here</a> to go back. Are you unable to finish the payment now? Then also do not worry, we have sent you an email containing a link that allows you to go to the payment page again. Do you not want to become a member of GEWIS after all? Let us know and we will delete your details. - - Was it not your intention to close the payment page? Then do not worry, we have sent you an e-mail containing a link that allows you to go to the payment page again. Do you not want to become a member of GEWIS after all? Let us know and we will delete your details. - Was it not your intention to close the payment page? Then do not worry, we have sent you an e-mail containing a link that allows you to go to the payment page again. Do you not want to become a member of GEWIS after all? Let us know and we will delete your details. + + Was it not your intention to close the payment page? Then do not worry, we have sent you an email containing a link that allows you to go to the payment page again. Do you not want to become a member of GEWIS after all? Let us know and we will delete your details. + Was it not your intention to close the payment page? Then do not worry, we have sent you an email containing a link that allows you to go to the payment page again. Do you not want to become a member of GEWIS after all? Let us know and we will delete your details. Was question %number%. @@ -11321,9 +11417,17 @@ We found a pointer to <code>NULL</code> instead of this page NULL instead of this page]]> - - We have received your registration, once your payment is verified the secretary will approve your membership. We will let you know via e-mail when this happens. - We have received your registration, once your payment is verified the secretary will approve your membership. We will let you know via e-mail when this happens. + + We have passed your request to have your data removed on to the secretary, who will be in touch. + We have passed your request to have your data removed on to the secretary, who will be in touch. + + + We have received your registration, once your payment is verified the secretary will approve your membership. We will let you know via email when this happens. + We have received your registration, once your payment is verified the secretary will approve your membership. We will let you know via email when this happens. + + + We have sent a message to your new email address. Your address changes once you have followed the link in it. + We have sent a message to your new email address. Your address changes once you have followed the link in it. We match you with a member who already passed the course. @@ -11761,6 +11865,10 @@ You are signed up. You are signed up. + + You are staying on as a graduate + You are staying on as a graduate + You are using administrator privileges to bypass maintenance mode; the website is closed to everyone else. You are using administrator privileges to bypass maintenance mode; the website is closed to everyone else. @@ -11881,6 +11989,10 @@ Your API token is "%token%". This value will NOT be shown again! Your API token is "%token%". This value will NOT be shown again! + + Your GEWIS membership ends on %date%. If you have finished studying, you can stay with the association as a graduate. + Your GEWIS membership ends on %date%. If you have finished studying, you can stay with the association as a graduate. + Your GEWIS registration Your GEWIS registration @@ -11985,6 +12097,10 @@ Your edit session expired; reopen the activity and try again. Your edit session expired; reopen the activity and try again. + + Your email address has been changed. You now sign in with it as well. + Your email address has been changed. You now sign in with it as well. + Your email address will expire Your email address will expire @@ -12009,6 +12125,14 @@ Your member name is never shown to other members. Your member name is never shown to other members. + + Your membership is ending + Your membership is ending + + + Your membership will end + Your membership will end + Your message is being sent to %count% recipient(s). Your message is being sent to %count% recipient(s). diff --git a/translations/messages.nl.xlf b/translations/messages.nl.xlf index 06d193734..4f74114c7 100644 --- a/translations/messages.nl.xlf +++ b/translations/messages.nl.xlf @@ -841,6 +841,10 @@ Accept your invitation Uitnodiging accepteren + + Accepted + Geaccepteerd + Access Denied Toegang geweigerd @@ -1349,6 +1353,10 @@ Already in the agenda Staat al in de agenda + + Also ask the secretary to remove my data + Vraag de secretaris ook om mijn gegevens te verwijderen + Also in another list Ook op een andere lijst @@ -1381,8 +1389,8 @@ An error occurred while we were trying to prepare the checkout page. Please try again later. Er is een fout opgetreden terwijl we probeerden de afrekenpagina voor te bereiden. Probeer het later nog eens. - - An error occurred while we were trying to prepare the checkout page. We have received your registration and sent you an e-mail containing a link that allows you to restart the checkout process at a later time. + + An error occurred while we were trying to prepare the checkout page. We have received your registration and sent you an email containing a link that allows you to restart the checkout process at a later time. Er is een fout opgetreden terwijl we probeerden de afrekenpagina voor te bereiden. We hebben je registratie ontvangen en je een e-mail gestuurd met een link waarmee je het betalingsproces op een later tijdstip opnieuw kunt starten. @@ -2129,6 +2137,10 @@ Change Subscriptions Abonnementen wijzigen + + Change email address + E-mailadres wijzigen + Change it Aanpassen @@ -2165,6 +2177,10 @@ Changing this asks the company for artwork at the new size; the banner it already has stays up until it hands one over. Dit wijzigen vraagt het bedrijf om beeldmateriaal op het nieuwe formaat; de banner die er al staat blijft staan tot het bedrijf een nieuwe aanlevert. + + Check the address we write to before you confirm: if it is your TU/e address, it stops working once you leave. + Controleer voordat je bevestigt naar welk adres we schrijven: als dat je TU/e-adres is, werkt het niet meer zodra je vertrekt. + Checkout Status Afrekenstatus @@ -2477,6 +2493,10 @@ Confirm sign-up Inschrijving bevestigen + + Confirm your email address + Bevestig je e-mailadres + Confirm your identity Bevestig je identiteit @@ -2941,6 +2961,10 @@ Declined Afgewezen + + Declined, removal requested + Afgewezen, verwijdering aangevraagd + Default Standaard @@ -3213,6 +3237,10 @@ Doing nothing is also an answer: your graduate status simply expires on the date shown below. Niets doen is ook een antwoord: je afgestudeerdenstatus verloopt gewoon op de hieronder getoonde datum. + + Doing nothing is also an answer: your membership simply ends on the date above. + Niets doen is ook een antwoord: je lidmaatschap eindigt dan gewoon op de datum hierboven. + Done Klaar @@ -3305,10 +3333,6 @@ Dutch Description Nederlandse beschrijving - - E-mail Address - E-mailadres - ES512 ES512 @@ -3441,6 +3465,10 @@ Email %count% selected E-mail %count% geselecteerden + + Email Address + E-mailadres + Email address E-mailadres @@ -3517,6 +3545,10 @@ End date and time Einddatum en -tijd + + End my membership + Beëindig mijn lidmaatschap + End sessions Sessies beëindigen @@ -4669,10 +4701,18 @@ If you hide yours, you also stop seeing others' age. The board can always see it. Als je die van jou verbergt, zie je ook de leeftijd van anderen niet meer. Het bestuur kan het altijd zien. + + If you would rather leave + Als je liever vertrekt + If you would rather remember it, take several random words and add characters that are not words. Take the words at random, not a phrase about yourself. %example% has leaked, so pick your own. See %link%. Wil je het liever onthouden, neem dan meerdere willekeurige woorden en voeg tekens toe die geen woord zijn. Kies de woorden willekeurig, niet een zin over jezelf. %example% is uitgelekt, kies dus je eigen woorden. Zie %link%. + + If you would still like to stay on as a graduate, let the secretary know. + Wil je toch afgestudeerde blijven, laat het de secretaris weten. + If your browser does not redirect you, %link% manually. Als je browser je niet doorstuurt, %link% dan handmatig. @@ -5373,9 +5413,9 @@ Mailing lists - List mailing lists Mailinglijsten - Mailinglijsten tonen - - Mailing lists - List subscribers of a mailing list, including their e-mail addresses - Mailinglijsten - Abonnees van een mailinglijst tonen, inclusief hun e-mailadressen + + Mailing lists - List subscribers of a mailing list, including their email addresses + Mailinglijsten - Abonnees van een mailinglijst tonen, inclusief hun e-mailadressen Mailman Mailing List @@ -5617,6 +5657,10 @@ Members who are still studying become ordinary. Leden die nog studeren worden gewoon lid. + + Members who have been asked themselves whether they want to stay on as a graduate are left out until their own offer expires unanswered. + Leden aan wie zelf is gevraagd of ze afgestudeerde willen blijven, blijven buiten de selectie totdat hun eigen aanbod onbeantwoord verloopt. + Members' accounts Ledenaccounts @@ -5637,6 +5681,10 @@ Membership Type (today) Lidmaatschapstype (huidig) + + Membership details + Lidmaatschapsgegevens + Membership number Lidmaatschapsnummer @@ -5917,12 +5965,12 @@ Name this query Geef deze query een naam - - Name, e-mail address or student number + + Name, email address or student number Naam, e-mailadres of studentnummer - - Name, e-mail address, member number or student number + + Name, email address, member number or student number Naam, e-mailadres, lidnummer of studentnummer @@ -5977,6 +6025,10 @@ New backup codes were generated for your account from %name%. Er zijn nieuwe back-upcodes gegenereerd voor je account vanaf %name%. + + New email address + Nieuw e-mailadres + New expiration Nieuwe afloopdatum @@ -6165,14 +6217,14 @@ No documents were uploaded under this agenda point. Er zijn geen documenten geüpload onder dit agendapunt. - - No e-mail on record - Geen e-mailadres geregistreerd - No email address Geen e-mailadres + + No email on record + Geen e-mailadres geregistreerd + No end time; meetings run until closed. Geen eindtijd; vergaderingen duren tot ze gesloten worden. @@ -6441,6 +6493,10 @@ Not activated Niet geactiveerd + + Not answered + Niet beantwoord + Not answered by me Niet door mij beantwoord @@ -8761,6 +8817,10 @@ Settings Instellingen + + Settled by the secretary + Afgehandeld door de secretaris + Shared by a member Gedeeld door een lid @@ -9149,10 +9209,18 @@ Status Status + + Stay on as a graduate + Afgestudeerde blijven + Staying at the university Blijven aan de universiteit + + Staying on as a graduate + Afgestudeerde blijven + Stemcommissie Stemcommissie @@ -9373,6 +9441,10 @@ Taking proposals Neemt voorstellen aan + + Tell us and we stop asking. Your membership ends on the date above either way. + Laat het ons weten, dan vragen we er niet meer naar. Je lidmaatschap eindigt hoe dan ook op de datum hierboven. + Terminate Beëindigen @@ -9405,10 +9477,22 @@ Thank you %name%! Bedankt, %name%! - - Thank you %name%! We have sent you a confirmation by e-mail. + + Thank you %name%! We have sent you a confirmation by email. Bedankt %name%! We hebben je een bevestiging gestuurd per e-mail. + + Thank you %name%! Your membership continues as a graduate membership. + Bedankt %name%! Je lidmaatschap loopt door als lidmaatschap voor afgestudeerden. + + + Thank you for letting us know, %name%. Your membership ends on %date% and we will not ask you about it again. + Bedankt voor het doorgeven, %name%. Je lidmaatschap eindigt op %date% en we vragen er niet meer naar. + + + Thank you for the years you were with us. + Bedankt voor de jaren dat je bij ons was. + That action is not available for this revision. Die actie is niet beschikbaar voor deze revisie. @@ -10005,6 +10089,10 @@ The round is open. Every body may propose without anything being set up for it. De ronde is open. Elk orgaan mag voorstellen zonder dat er iets voor is ingesteld. + + The secretary keeps these. Write to them if any of it is wrong. + De secretaris houdt deze gegevens bij. Neem contact met hen op als er iets niet klopt. + The selected photos are already in that album. De geselecteerde foto's staan al in dat album. @@ -10085,8 +10173,8 @@ Theme Thema - - There already is a member with this e-mail address. + + There already is a member with this email address. Er is al een lid met dit e-mailadres. @@ -10405,6 +10493,10 @@ This is %source%. Dit is %source%. + + This is already your email address. + Dit is al je e-mailadres. + This is the address the association writes to, and the one you sign in with. Dit is het adres waar de vereniging naartoe schrijft, en waarmee je inlogt. @@ -11025,6 +11117,10 @@ Use %number% Gebruik %number% + + Use an address you keep after you stop studying. + Gebruik een adres dat je houdt nadat je stopt met studeren. + Use “quotes” for exact phrases, -word to exclude, type:bm to limit to one meeting type, and meeting:1805 to limit to one meeting. Gebruik “aanhalingstekens” voor exacte zinnen, -woord om uit te sluiten, type:bm om te beperken tot een vergadertype en meeting:1805 om te beperken tot een vergadering. @@ -11301,12 +11397,12 @@ Was Was - - Was it not your intention to close the payment page? Then do not worry, <a href="%url%">click here</a> to go back. Are you unable to finish the payment now? Then also do not worry, we have sent you an e-mail containing a link that allows you to go to the payment page again. Do you not want to become a member of GEWIS after all? Let us know and we will delete your details. + + Was it not your intention to close the payment page? Then do not worry, <a href="%url%">click here</a> to go back. Are you unable to finish the payment now? Then also do not worry, we have sent you an email containing a link that allows you to go to the payment page again. Do you not want to become a member of GEWIS after all? Let us know and we will delete your details. klik hier om terug te gaan. Lukt het je nu niet om de betaling af te ronden? Ook geen zorgen, we hebben je een e-mail gestuurd met een link waarmee je opnieuw naar de betaalpagina kunt. Wil je toch geen lid worden van GEWIS? Laat het ons weten, dan verwijderen we je gegevens.]]> - - Was it not your intention to close the payment page? Then do not worry, we have sent you an e-mail containing a link that allows you to go to the payment page again. Do you not want to become a member of GEWIS after all? Let us know and we will delete your details. + + Was it not your intention to close the payment page? Then do not worry, we have sent you an email containing a link that allows you to go to the payment page again. Do you not want to become a member of GEWIS after all? Let us know and we will delete your details. Was het niet je bedoeling om de betaalpagina te sluiten? Geen zorgen, we hebben je een e-mail gestuurd met een link waarmee je opnieuw naar de betaalpagina kunt gaan. Wil je toch geen lid worden van GEWIS? Laat het ons weten en we verwijderen jouw gegevens. @@ -11321,10 +11417,18 @@ We found a pointer to <code>NULL</code> instead of this page NULL gevonden in plaats van deze pagina]]> - - We have received your registration, once your payment is verified the secretary will approve your membership. We will let you know via e-mail when this happens. + + We have passed your request to have your data removed on to the secretary, who will be in touch. + We hebben je verzoek om je gegevens te verwijderen doorgegeven aan de secretaris, die contact met je opneemt. + + + We have received your registration, once your payment is verified the secretary will approve your membership. We will let you know via email when this happens. We hebben je aanmelding ontvangen, zodra je betaling is geverifieerd zal de secretaris je lidmaatschap goedkeuren. We laten je via e-mail weten wanneer dit is gebeurd. + + We have sent a message to your new email address. Your address changes once you have followed the link in it. + We hebben een bericht naar je nieuwe e-mailadres gestuurd. Je adres verandert zodra je de link daarin hebt gevolgd. + We match you with a member who already passed the course. We koppelen je aan een lid dat het vak al gehaald heeft. @@ -11761,6 +11865,10 @@ You are signed up. Je bent ingeschreven. + + You are staying on as a graduate + Je blijft afgestudeerde + You are using administrator privileges to bypass maintenance mode; the website is closed to everyone else. Je gebruikt beheerdersprivileges om de onderhoudsmodus te omzeilen; de website is gesloten voor iedereen anders. @@ -11881,6 +11989,10 @@ Your API token is "%token%". This value will NOT be shown again! Je API-token is "%token%". Deze waarde wordt NIET nog een keer getoond! + + Your GEWIS membership ends on %date%. If you have finished studying, you can stay with the association as a graduate. + Je GEWIS-lidmaatschap eindigt op %date%. Als je klaar bent met studeren, kun je als afgestudeerde bij de vereniging blijven. + Your GEWIS registration Je GEWIS-inschrijving @@ -11985,6 +12097,10 @@ Your edit session expired; reopen the activity and try again. Je bewerkingssessie is verlopen; open de activiteit opnieuw en probeer het nogmaals. + + Your email address has been changed. You now sign in with it as well. + Je e-mailadres is gewijzigd. Je logt er nu ook mee in. + Your email address will expire Je e-mailadres komt te vervallen @@ -12009,6 +12125,14 @@ Your member name is never shown to other members. Je lidnaam wordt nooit aan andere leden getoond. + + Your membership is ending + Je lidmaatschap loopt af + + + Your membership will end + Je lidmaatschap eindigt + Your message is being sent to %count% recipient(s). Je bericht wordt verzonden naar %count% ontvanger(s). diff --git a/translations/validators.en.xlf b/translations/validators.en.xlf index 0f3b63fda..a16a0ac86 100644 --- a/translations/validators.en.xlf +++ b/translations/validators.en.xlf @@ -545,9 +545,9 @@ You cannot propose an activity for this body. You cannot propose an activity for this body. - - You cannot use your TU/e (student) e-mail address because if you leave or stop studying, we can no longer reach you about important announcements. - You cannot use your TU/e (student) e-mail address because if you leave or stop studying, we can no longer reach you about important announcements. + + You cannot use your TU/e (student) email address because if you leave or stop studying, we can no longer reach you about important announcements. + You cannot use your TU/e (student) email address because if you leave or stop studying, we can no longer reach you about important announcements. You have to agree to the Articles of Association and the Internal Regulations diff --git a/translations/validators.nl.xlf b/translations/validators.nl.xlf index 5a5d1cb4b..9664502d4 100644 --- a/translations/validators.nl.xlf +++ b/translations/validators.nl.xlf @@ -545,8 +545,8 @@ You cannot propose an activity for this body. Je kunt geen activiteit voorstellen voor dit orgaan. - - You cannot use your TU/e (student) e-mail address because if you leave or stop studying, we can no longer reach you about important announcements. + + You cannot use your TU/e (student) email address because if you leave or stop studying, we can no longer reach you about important announcements. Je kunt je TU/e-(studenten)e-mailadres niet gebruiken, want als je vertrekt of stopt met studeren kunnen we je niet meer bereiken over belangrijke mededelingen. From 7b3916732939bee26abd51b59513f12962d2d1ca Mon Sep 17 00:00:00 2001 From: Tom Udding Date: Sun, 30 Aug 2026 16:49:29 +0200 Subject: [PATCH 3/5] fix(checker): bound the graduate renewal sweep at the far end --- src/Entity/Database/RenewalLink.php | 7 ++++++- src/Repository/Checker/MemberRepository.php | 9 +++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Entity/Database/RenewalLink.php b/src/Entity/Database/RenewalLink.php index 45df6ea5a..18105f9ce 100644 --- a/src/Entity/Database/RenewalLink.php +++ b/src/Entity/Database/RenewalLink.php @@ -20,6 +20,11 @@ #[Entity(repositoryClass: RenewalLinkRepository::class)] class RenewalLink extends ActionLink { + /** + * Also how far back the sweep that sends these is willing to look. + */ + public const int GRACE_DAYS = 30; + /** * The member */ @@ -72,6 +77,6 @@ public function linkExpired(): bool { $diff = new DateTimeImmutable()->diff($this->currentExpiration); - return 1 === $diff->invert && ($diff->days > 30); + return 1 === $diff->invert && ($diff->days > self::GRACE_DAYS); } } diff --git a/src/Repository/Checker/MemberRepository.php b/src/Repository/Checker/MemberRepository.php index 9906fb9f6..8c36f63e5 100644 --- a/src/Repository/Checker/MemberRepository.php +++ b/src/Repository/Checker/MemberRepository.php @@ -54,6 +54,15 @@ public function getExpiringGraduates( ->andWhere('m.deleted = false') ->andWhere($qb->expr()->eq('mem.startDate', '(' . $this->lastMembershipQuery()->getDQL() . ')')) ->andWhere('mem.endDate <= :expiresBefore') + // Bounded at the far end, or the sweep works back through every graduate who ever expired. + ->andWhere('mem.endDate >= :expiresAfter') + ->setParameter( + 'expiresAfter', + new DateTimeImmutable()->modify('-' . RenewalLink::GRACE_DAYS . ' days')->setTime( + 0, + 0, + ), + ) ->setParameter( 'graduate', MembershipTypes::Graduate, From 76bb9433c694eb9d4fb97904a679e75d2691b4fa Mon Sep 17 00:00:00 2001 From: Tom Udding Date: Sun, 30 Aug 2026 14:59:22 +0200 Subject: [PATCH 4/5] feat(database): show the memberships a member has held For sake of transparency, show how the memberhsip of someone changed each year. --- templates/database/member/show.html.twig | 12 ++++++ .../partials/membership-history.html.twig | 38 +++++++++++++++++++ templates/user/settings/details.html.twig | 6 ++- .../User/MemberDetailsControllerTest.php | 13 +++++++ translations/messages.en.xlf | 8 ++++ translations/messages.nl.xlf | 8 ++++ 6 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 templates/partials/membership-history.html.twig diff --git a/templates/database/member/show.html.twig b/templates/database/member/show.html.twig index 260776064..e7d4ad85b 100644 --- a/templates/database/member/show.html.twig +++ b/templates/database/member/show.html.twig @@ -142,6 +142,18 @@
    +
    +
    +

    {{ 'Memberships'|trans }}

    +
    +
    + {% include 'partials/membership-history.html.twig' with { + 'memberships': member.memberships, + 'show_paid': true, + } only %} +
    +
    +

    {{ 'Membership of Committees and Fraternities'|trans }}

    diff --git a/templates/partials/membership-history.html.twig b/templates/partials/membership-history.html.twig new file mode 100644 index 000000000..c4734333f --- /dev/null +++ b/templates/partials/membership-history.html.twig @@ -0,0 +1,38 @@ +{# The memberships a member has held, oldest first, which is how the register records the years somebody was with the + association. A membership always ends on a date, even the one that is running: what a renewal writes is the next + membership after it, so a row that ends today is not the same as a member who left. + + `show_paid` is for the register, where what was paid for a year is part of the record. It is not the member's own + business on their own page: what they paid they know, and it is the association's bookkeeping rather than theirs. #} +{% set show_paid = show_paid|default(false) %} + +{% if memberships is empty %} +

    {{ 'No memberships on record.'|trans }}

    +{% else %} +
    + + + + + + + {% if show_paid %} + + {% endif %} + + + + {% for membership in memberships %} + + + + + {% if show_paid %} + + {% endif %} + + {% endfor %} + +
    {{ 'Type'|trans }}{{ 'From'|trans }}{{ 'Until'|trans }}{{ 'Paid'|trans }}
    {{ membership.type|trans }}{{ membership.startDate|format_date(pattern: 'd MMMM y') }}{{ membership.endDate|format_date(pattern: 'd MMMM y') }}{{ membership.paid }}
    +
    +{% endif %} diff --git a/templates/user/settings/details.html.twig b/templates/user/settings/details.html.twig index e94943344..7bae0c251 100644 --- a/templates/user/settings/details.html.twig +++ b/templates/user/settings/details.html.twig @@ -132,9 +132,13 @@ {% endif %}
    -

    +

    {{ 'The secretary keeps these. Write to them if any of it is wrong.'|trans }}

    +

    {{ 'Your memberships'|trans }}

    + {% include 'partials/membership-history.html.twig' with { + 'memberships': member.memberships, + } only %}
    diff --git a/tests/Integration/Controller/User/MemberDetailsControllerTest.php b/tests/Integration/Controller/User/MemberDetailsControllerTest.php index 70526b4cd..509eac1cf 100644 --- a/tests/Integration/Controller/User/MemberDetailsControllerTest.php +++ b/tests/Integration/Controller/User/MemberDetailsControllerTest.php @@ -67,6 +67,19 @@ public function testShowsWhatAMemberMayChangeAndWhatTheSecretaryKeeps(): void $content, ); + // The years they were with the association, which is the record the secretary keeps and the member is shown. + self::assertNotCount( + 0, + $member->getMemberships(), + ); + + foreach ($member->getMemberships() as $membership) { + self::assertStringContainsString( + $membership->getStartDate()->format('Y'), + $content, + ); + } + foreach ($this->ledger->getRepository(MailingList::class)->findAll() as $list) { if ($list->selfService) { self::assertStringContainsString( diff --git a/translations/messages.en.xlf b/translations/messages.en.xlf index c54a46dfa..6eac85ca7 100644 --- a/translations/messages.en.xlf +++ b/translations/messages.en.xlf @@ -6317,6 +6317,10 @@ No membership expires soon and nothing is missing. No membership expires soon and nothing is missing.     + + No memberships on record. + No memberships on record. + No more results No more results @@ -12133,6 +12137,10 @@ Your membership will end Your membership will end + + Your memberships + Your memberships + Your message is being sent to %count% recipient(s). Your message is being sent to %count% recipient(s). diff --git a/translations/messages.nl.xlf b/translations/messages.nl.xlf index 4f74114c7..6787deb54 100644 --- a/translations/messages.nl.xlf +++ b/translations/messages.nl.xlf @@ -6317,6 +6317,10 @@ No membership expires soon and nothing is missing. Er verloopt binnenkort geen lidmaatschap en er ontbreekt niets. + + No memberships on record. + Geen lidmaatschappen geregistreerd. + No more results Geen resultaten meer @@ -12133,6 +12137,10 @@ Your membership will end Je lidmaatschap eindigt + + Your memberships + Jouw lidmaatschappen + Your message is being sent to %count% recipient(s). Je bericht wordt verzonden naar %count% ontvanger(s). From 02389d6e978b2ae111b57a24b2540e0e4d81749c Mon Sep 17 00:00:00 2001 From: Tom Udding Date: Sun, 30 Aug 2026 17:17:39 +0200 Subject: [PATCH 5/5] feat(checker): check that a member's memberships hold together Closes GH-66 --- config/reference.php | 4 +- .../CheckMembershipConsistencyCommand.php | 56 ++++++ .../Database/Enums/MembershipProblems.php | 37 ++++ src/Service/Checker/Membership.php | 116 +++++++++++++ src/ViewModel/Checker/MembershipError.php | 62 +++++++ .../checker/membership-report.txt.twig | 10 ++ .../Checker/MembershipConsistencyTest.php | 80 +++++++++ tests/Scheduler/MainScheduleTest.php | 1 + tests/Service/Checker/MembershipTest.php | 164 ++++++++++++++++++ translations/messages.en.xlf | 12 ++ translations/messages.nl.xlf | 12 ++ 11 files changed, 552 insertions(+), 2 deletions(-) create mode 100644 src/Command/Checker/CheckMembershipConsistencyCommand.php create mode 100644 src/Entity/Database/Enums/MembershipProblems.php create mode 100644 src/Service/Checker/Membership.php create mode 100644 src/ViewModel/Checker/MembershipError.php create mode 100644 templates/database/checker/membership-report.txt.twig create mode 100644 tests/Integration/Service/Checker/MembershipConsistencyTest.php create mode 100644 tests/Service/Checker/MembershipTest.php diff --git a/config/reference.php b/config/reference.php index 0004a15cb..d42deda65 100644 --- a/config/reference.php +++ b/config/reference.php @@ -1846,8 +1846,8 @@ * } * @psalm-type MercureConfig = array{ * hubs?: arrayrunExclusively( + $output, + fn (): int => $this->executeExclusively( + $input, + $output, + ), + ); + } + + private function executeExclusively( + InputInterface $input, + OutputInterface $output, + ): int { + $this->membershipService->check(); + + return Command::SUCCESS; + } +} diff --git a/src/Entity/Database/Enums/MembershipProblems.php b/src/Entity/Database/Enums/MembershipProblems.php new file mode 100644 index 000000000..3dcf68479 --- /dev/null +++ b/src/Entity/Database/Enums/MembershipProblems.php @@ -0,0 +1,37 @@ + new TranslatableMessage('A membership ends before it starts'), + self::Overlapping => new TranslatableMessage('Two memberships cover the same days'), + self::StartsOnTheSameDay => new TranslatableMessage('Two memberships start on the same day'), + }; + } + + #[Override] + public function trans( + TranslatorInterface $translator, + ?string $locale = null, + ): string { + return $this->getName()->trans( + $translator, + $locale, + ); + } +} diff --git a/src/Service/Checker/Membership.php b/src/Service/Checker/Membership.php new file mode 100644 index 000000000..f8492fe1a --- /dev/null +++ b/src/Service/Checker/Membership.php @@ -0,0 +1,116 @@ +memberRepository->findAll() as $member) { + foreach ($this->problemsOf($member) as $error) { + $errors[] = $error; + } + } + + $this->sendMail($errors); + } + + /** + * A gap is not a problem: somebody can leave the association and come back. + * + * @return MembershipError[] + */ + public function problemsOf(MemberModel $member): array + { + $memberships = $member->getMemberships()->toArray(); + + usort( + $memberships, + static fn (MembershipModel $a, MembershipModel $b): int => $a->getStartDate() <=> $b->getStartDate(), + ); + + $errors = []; + $previous = null; + + foreach ($memberships as $membership) { + if ($membership->endDate < $membership->getStartDate()) { + $errors[] = new MembershipError( + $member, + MembershipProblems::EndsBeforeItStarts, + $membership, + ); + } + + if (null !== $previous) { + if ( + $previous->getStartDate()->getTimestamp() === $membership->getStartDate()->getTimestamp() + ) { + $errors[] = new MembershipError( + $member, + MembershipProblems::StartsOnTheSameDay, + $previous, + $membership, + ); + } elseif ($previous->endDate > $membership->getStartDate()) { + // Which of the two is the member's on the days both cover is not something the record answers. + $errors[] = new MembershipError( + $member, + MembershipProblems::Overlapping, + $previous, + $membership, + ); + } + } + + $previous = $membership; + } + + return $errors; + } + + /** + * Sent even when there is nothing to report, so a silent week is a week that was checked. + * + * @param MembershipError[] $errors + */ + private function sendMail(array $errors): void + { + $message = new TemplatedEmail() + ->to(new Address($this->mailToCheckerResultAddress, $this->mailToCheckerResultName)) + ->from(new Address($this->mailFromAddress, $this->mailFromName)) + ->subject('Membership Checker Report') + ->textTemplate('database/checker/membership-report.txt.twig') + ->context([ + 'errors' => $errors, + 'count' => count($errors), + ]); + + $this->mailer->send($message); + } +} diff --git a/src/ViewModel/Checker/MembershipError.php b/src/ViewModel/Checker/MembershipError.php new file mode 100644 index 000000000..bd2eb587a --- /dev/null +++ b/src/ViewModel/Checker/MembershipError.php @@ -0,0 +1,62 @@ +member; + } + + public function getProblem(): MembershipProblems + { + return $this->problem; + } + + public function asText(): string + { + $membership = $this->describe($this->membership); + + if (null !== $this->other) { + $membership .= ' and ' . $this->describe($this->other); + } + + return sprintf( + '%s: %d (%s) for %s', + $this->problem->getName()->getMessage(), + $this->member->lidnr, + $this->member->getFullName(), + $membership, + ); + } + + private function describe(MembershipModel $membership): string + { + return sprintf( + '%s %s to %s', + $membership->type->value, + $membership->getStartDate()->format('Y-m-d'), + $membership->endDate->format('Y-m-d'), + ); + } +} diff --git a/templates/database/checker/membership-report.txt.twig b/templates/database/checker/membership-report.txt.twig new file mode 100644 index 000000000..a02c416d7 --- /dev/null +++ b/templates/database/checker/membership-report.txt.twig @@ -0,0 +1,10 @@ +{# `errors` holds one line per thing that does not hold about one member's memberships. A `.txt.twig` template is not + escaped, so every line arrives as `App\ViewModel\Checker\MembershipError::asText()` wrote it. #} +{% if 0 == count %} +Every member's memberships run in order and do not overlap. +{% else %} +{{ count }} membership(s) do not hold: +{% for error in errors %} +{{ error.asText }} +{% endfor %} +{% endif %} diff --git a/tests/Integration/Service/Checker/MembershipConsistencyTest.php b/tests/Integration/Service/Checker/MembershipConsistencyTest.php new file mode 100644 index 000000000..2779ecffe --- /dev/null +++ b/tests/Integration/Service/Checker/MembershipConsistencyTest.php @@ -0,0 +1,80 @@ +get('doctrine')->getManager('default'); + self::assertInstanceOf( + EntityManagerInterface::class, + $ledger, + ); + + $this->ledger = $ledger; + } + + public function testTheSeededRegisterHoldsTogether(): void + { + $service = self::getContainer()->get(MembershipService::class); + $problems = []; + + foreach ($this->ledger->getRepository(Member::class)->findAll() as $member) { + foreach ($service->problemsOf($member) as $problem) { + $problems[] = $problem->asText(); + } + } + + self::assertSame( + [], + $problems, + "The seed writes memberships that overlap or run backwards:\n" . implode( + "\n", + $problems, + ), + ); + } + + public function testWritesToTheSecretaryEvenWhenNothingIsWrong(): void + { + self::getContainer()->get(MembershipService::class)->check(); + + self::assertEmailCount(1); + $email = self::getMailerMessage(); + self::assertInstanceOf( + Email::class, + $email, + ); + + self::assertSame( + 'Membership Checker Report', + $email->getSubject(), + ); + self::assertStringContainsString( + 'run in order and do not overlap', + (string) $email->getTextBody(), + ); + } +} diff --git a/tests/Scheduler/MainScheduleTest.php b/tests/Scheduler/MainScheduleTest.php index 2647d8b4f..5f7d5ec02 100644 --- a/tests/Scheduler/MainScheduleTest.php +++ b/tests/Scheduler/MainScheduleTest.php @@ -58,6 +58,7 @@ class MainScheduleTest extends KernelTestCase 'app:user:update-ip-databases' => 'maintenance', 'app:users:force-relogin' => 'cron', 'check:database' => 'maintenance', + 'check:membership:consistency' => 'maintenance', 'check:membership:conversion:graduate' => 'maintenance', 'check:membership:renewal:graduate' => 'maintenance', 'database:mailinglist:fetch all' => 'maintenance', diff --git a/tests/Service/Checker/MembershipTest.php b/tests/Service/Checker/MembershipTest.php new file mode 100644 index 000000000..2bbfccabe --- /dev/null +++ b/tests/Service/Checker/MembershipTest.php @@ -0,0 +1,164 @@ +member([ + [ + '2018-07-01', + '2019-07-01', + ], + [ + '2019-07-01', + '2020-07-01', + ], + [ + '2020-07-01', + '2021-07-01', + ], + ]); + + self::assertSame( + [], + $this->service()->problemsOf($member), + ); + } + + public function testAcceptsAGapBetweenTwoMemberships(): void + { + $member = $this->member([ + [ + '2018-07-01', + '2019-07-01', + ], + [ + '2024-07-01', + '2025-07-01', + ], + ]); + + self::assertSame( + [], + $this->service()->problemsOf($member), + ); + } + + public function testReportsTwoMembershipsCoveringTheSameDays(): void + { + $member = $this->member([ + [ + '2018-07-01', + '2020-07-01', + ], + [ + '2019-07-01', + '2021-07-01', + ], + ]); + + self::assertSame( + [MembershipProblems::Overlapping], + $this->problems($member), + ); + } + + public function testReportsTwoMembershipsStartingOnTheSameDay(): void + { + $member = $this->member([ + [ + '2019-07-01', + '2020-07-01', + ], + [ + '2019-07-01', + '2020-07-01', + ], + ]); + + self::assertSame( + [MembershipProblems::StartsOnTheSameDay], + $this->problems($member), + ); + } + + public function testReadsMembershipsInTheOrderTheyRun(): void + { + $member = $this->member([ + [ + '2020-07-01', + '2021-07-01', + ], + [ + '2018-07-01', + '2019-07-01', + ], + ]); + + self::assertSame( + [], + $this->service()->problemsOf($member), + ); + } + + /** + * @return MembershipProblems[] + */ + private function problems(Member $member): array + { + return array_map( + static fn (MembershipError $error): MembershipProblems => $error->getProblem(), + $this->service()->problemsOf($member), + ); + } + + private function service(): MembershipService + { + return new MembershipService( + self::createStub(MemberRepository::class), + self::createStub(MailerInterface::class), + 'from@example.org', + 'From', + 'to@example.org', + 'To', + ); + } + + /** + * @param array $memberships + */ + private function member(array $memberships): Member + { + $member = new Member(); + + foreach ($memberships as [$start, $end]) { + $member->addMembership(new Membership( + $member, + MembershipTypes::Ordinary, + new DateTimeImmutable($start), + new DateTimeImmutable($end), + )); + } + + return $member; + } +} diff --git a/translations/messages.en.xlf b/translations/messages.en.xlf index 6eac85ca7..073f22faf 100644 --- a/translations/messages.en.xlf +++ b/translations/messages.en.xlf @@ -669,6 +669,10 @@ A mailing list sync is running A mailing list sync is running + + A membership ends before it starts + A membership ends before it starts + A new activity "%name%" has been published. A new activity "%name%" has been published. @@ -10905,6 +10909,14 @@ Turn this proposal down? Turn this proposal down? + + Two memberships cover the same days + Two memberships cover the same days + + + Two memberships start on the same day + Two memberships start on the same day + Two windows, and they are not the same: when bodies may hand a proposal in, and which days the activities they propose may fall on. Handing in usually happens well before the quartile itself. Two windows, and they are not the same: when bodies may hand a proposal in, and which days the activities they propose may fall on. Handing in usually happens well before the quartile itself. diff --git a/translations/messages.nl.xlf b/translations/messages.nl.xlf index 6787deb54..141e3fea1 100644 --- a/translations/messages.nl.xlf +++ b/translations/messages.nl.xlf @@ -669,6 +669,10 @@ A mailing list sync is running Er loopt een mailinglijstsynchronisatie + + A membership ends before it starts + Een lidmaatschap eindigt voordat het begint + A new activity "%name%" has been published. Een nieuwe activiteit "%name%" is gepubliceerd. @@ -10905,6 +10909,14 @@ Turn this proposal down? Dit voorstel afwijzen? + + Two memberships cover the same days + Twee lidmaatschappen beslaan dezelfde dagen + + + Two memberships start on the same day + Twee lidmaatschappen beginnen op dezelfde dag + Two windows, and they are not the same: when bodies may hand a proposal in, and which days the activities they propose may fall on. Handing in usually happens well before the quartile itself. Twee vensters, en dat is niet hetzelfde: wanneer organen een voorstel mogen indienen, en op welke dagen de activiteiten die ze voorstellen mogen vallen. Indienen gebeurt meestal ruim voor het kwartiel zelf.