From d654ce77fb8a93a4e5dcee74a88477bf86ca354b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Tue, 11 Aug 2026 10:21:25 +0200 Subject: [PATCH] feat: soft-delete for teamer accounts addresses #869dv9br3 --- migrations/Version20260811075140.php | 28 ++++ src/BusProNet/UserDataHandler.php | 13 ++ .../Admin/Teamer/DeleteAccountController.php | 62 ++++++++ .../Profile/DeleteAccountController.php | 45 ++++++ src/Entity/SoftDeletableEntityInterface.php | 2 +- src/Entity/Teamer.php | 4 +- src/Entity/Traits/SoftDeletableEntity.php | 9 +- src/Entity/User.php | 20 ++- src/EventListener/DeletedUserSubscriber.php | 54 +++++++ .../EmailNotificationSubscriber.php | 38 +++++ .../RequiredTeamerCheckSubscriber.php | 3 + src/Form/DeleteAccountType.php | 26 +++ src/Form/TeamerFilterType.php | 4 + src/Model/TeamerFilterDto.php | 13 ++ src/Repository/AvailabilityRepository.php | 11 +- src/Repository/TeamerRepository.php | 10 +- src/Repository/UserRepository.php | 5 + src/Security/BpnAuthenticator.php | 13 ++ src/Security/UserChecker.php | 6 + src/Security/Voter/ImpersonationVoter.php | 6 + src/Service/Common/TeamerFilterHandler.php | 5 + .../Cron/DispositionReminderService.php | 2 + src/Service/Cron/UploadReminderService.php | 6 + src/Service/Teamer/AccountDeletionHandler.php | 98 ++++++++++++ src/Validator/Constraints/Application.php | 2 + .../Constraints/ApplicationValidator.php | 12 ++ .../_partials/_disposition_data.html.twig | 1 + .../_partials/_teamer_deleted_badge.html.twig | 6 + templates/admin/application/index.html.twig | 1 + .../teamer/modal_delete_account.html.twig | 33 ++++ .../teamer/modal_restore_account.html.twig | 17 ++ .../assignment/detail.html.twig | 2 + .../administrative/document/index.html.twig | 1 + .../administrative/feedback/index.html.twig | 1 + .../administrative/teamer/index.html.twig | 31 +++- .../common/modal_teamer_filter.html.twig | 1 + templates/teamer/profile/index.html.twig | 17 ++ .../profile/modal_delete_account.html.twig | 27 ++++ tests/BusProNet/UserDataHandlerTest.php | 67 ++++++++ .../Entity/Traits/SoftDeletableEntityTest.php | 55 +++++++ tests/Entity/UserTest.php | 39 +++++ .../EmailNotificationSubscriberTest.php | 107 +++++++++++++ tests/Security/BpnAuthenticatorTest.php | 38 +++++ tests/Security/UserCheckerTest.php | 69 ++++++++ .../Teamer/AccountDeletionHandlerTest.php | 148 ++++++++++++++++++ 45 files changed, 1151 insertions(+), 7 deletions(-) create mode 100644 migrations/Version20260811075140.php create mode 100644 src/Controller/Admin/Teamer/DeleteAccountController.php create mode 100644 src/Controller/Teamer/Profile/DeleteAccountController.php create mode 100644 src/EventListener/DeletedUserSubscriber.php create mode 100644 src/Form/DeleteAccountType.php create mode 100644 src/Service/Teamer/AccountDeletionHandler.php create mode 100644 templates/_partials/_teamer_deleted_badge.html.twig create mode 100644 templates/admin/teamer/modal_delete_account.html.twig create mode 100644 templates/admin/teamer/modal_restore_account.html.twig create mode 100644 templates/teamer/profile/modal_delete_account.html.twig create mode 100644 tests/Entity/Traits/SoftDeletableEntityTest.php create mode 100644 tests/EventListener/EmailNotificationSubscriberTest.php create mode 100644 tests/Security/UserCheckerTest.php create mode 100644 tests/Service/Teamer/AccountDeletionHandlerTest.php diff --git a/migrations/Version20260811075140.php b/migrations/Version20260811075140.php new file mode 100644 index 0000000..c04ee57 --- /dev/null +++ b/migrations/Version20260811075140.php @@ -0,0 +1,28 @@ +addSql('ALTER TABLE teamer ADD deleted_at DATETIME DEFAULT NULL COMMENT \'(DC2Type:datetime_immutable)\''); + $this->addSql('ALTER TABLE user ADD deleted_at DATETIME DEFAULT NULL COMMENT \'(DC2Type:datetime_immutable)\''); + } + + public function down(Schema $schema): void + { + $this->addSql('ALTER TABLE teamer DROP deleted_at'); + $this->addSql('ALTER TABLE user DROP deleted_at'); + } +} diff --git a/src/BusProNet/UserDataHandler.php b/src/BusProNet/UserDataHandler.php index a635f64..fdbe706 100644 --- a/src/BusProNet/UserDataHandler.php +++ b/src/BusProNet/UserDataHandler.php @@ -97,6 +97,13 @@ class UserDataHandler /** @var User $user */ $user = $users[0]; + // A deleted account still has to be matched here, otherwise the caller would take + // it for an unknown person and create a second account for them - resurrecting + // them under a new row. It is returned as found, without writing anything to it. + if (true === $user->isDeleted()) { + return $user; + } + $addressId = $profileResponse->getAddressId(); $personId = $profileResponse->getPersonId(); if (null === $addressId || null === $personId) { @@ -232,6 +239,12 @@ class UserDataHandler */ public function disableForRevokedCrmRoles(User $user): void { + // a deleted account is already excluded from everything and must not be written + // to by the CRM at all + if (true === $user->isDeleted()) { + return; + } + // an existing block may be a disciplinary one and must never be overwritten, but // findLocalUser() may have refreshed the BusPro ids and nothing else flushes here if (true === $user->isDisabled()) { diff --git a/src/Controller/Admin/Teamer/DeleteAccountController.php b/src/Controller/Admin/Teamer/DeleteAccountController.php new file mode 100644 index 0000000..e503d5c --- /dev/null +++ b/src/Controller/Admin/Teamer/DeleteAccountController.php @@ -0,0 +1,62 @@ +isMethod(Request::METHOD_POST)) { + $this->accountDeletionHandler->delete($teamer, AccountDeletionHandler::SOURCE_ADMIN); + + $this->addFlash('success', 'Der Account wurde gelöscht'); + + return new HxRedirectResponse($this->generateUrl('app_administrative_teamer_index')); + } + + // open commitments survive the deletion and stay visible, but the teamer is no + // longer written to about them - withdrawing them is the admin's decision + return $this->render('admin/teamer/modal_delete_account.html.twig', [ + 'teamer' => $teamer, + 'pendingApplicationCount' => count($this->applicationRepository->getPendingForTeamer($teamer, 100)), + 'upcomingDispositionCount' => count($this->dispositionRepository->getUpcomingDispositionsByTeamer($teamer, 100)), + ]); + } + + #[Route('/admin/teamer/restore-account/{uuid}', name: 'app_admin_teamer_restore_account')] + #[IsGranted('ROLE_ADMIN')] + public function restore(Teamer $teamer, Request $request): Response + { + if (true === $request->isMethod(Request::METHOD_POST)) { + $this->accountDeletionHandler->restore($teamer); + + $this->addFlash('success', 'Der Account wurde wiederhergestellt'); + + return new HxRedirectResponse($this->generateUrl('app_administrative_teamer_index')); + } + + return $this->render('admin/teamer/modal_restore_account.html.twig', [ + 'teamer' => $teamer, + ]); + } +} diff --git a/src/Controller/Teamer/Profile/DeleteAccountController.php b/src/Controller/Teamer/Profile/DeleteAccountController.php new file mode 100644 index 0000000..bef3909 --- /dev/null +++ b/src/Controller/Teamer/Profile/DeleteAccountController.php @@ -0,0 +1,45 @@ +getUser(); + + $form = $this->createForm(DeleteAccountType::class, null, ['hx_post' => $request->getUri()]); + $form->handleRequest($request); + + if ($form->isSubmitted() && $form->isValid()) { + $this->accountDeletionHandler->delete($user, AccountDeletionHandler::SOURCE_SELF); + + $this->addFlash('success', 'Dein Account wurde gelöscht'); + + // the session stays authenticated until the next request, so the logout is + // done here rather than left to the DeletedUserSubscriber + return new HxRedirectResponse($this->generateUrl('app_security_logout')); + } + + return $this->render('teamer/profile/modal_delete_account.html.twig', [ + 'form' => $form->createView(), + ]); + } +} diff --git a/src/Entity/SoftDeletableEntityInterface.php b/src/Entity/SoftDeletableEntityInterface.php index 86a82dc..73d5d57 100644 --- a/src/Entity/SoftDeletableEntityInterface.php +++ b/src/Entity/SoftDeletableEntityInterface.php @@ -6,5 +6,5 @@ interface SoftDeletableEntityInterface { public function getDeletedAt(): ?\DateTimeImmutable; - public function setDeletedAt(\DateTimeImmutable $createdAt): static; + public function setDeletedAt(?\DateTimeImmutable $deletedAt): static; } diff --git a/src/Entity/Teamer.php b/src/Entity/Teamer.php index b3543c6..9a32cd4 100644 --- a/src/Entity/Teamer.php +++ b/src/Entity/Teamer.php @@ -6,6 +6,7 @@ use App\BusProNet\Model\ProfileResponse; use App\Entity\Embeddable\Address; use App\Entity\Embeddable\BankAccount; use App\Entity\Embeddable\Communication; +use App\Entity\Traits\SoftDeletableEntity; use App\Entity\Traits\TimestampableEntity; use App\Repository\TeamerRepository; use Doctrine\Common\Collections\ArrayCollection; @@ -16,9 +17,10 @@ use Symfony\Component\Uid\Uuid; use Symfony\Component\Validator\Constraints as Assert; #[ORM\Entity(repositoryClass: TeamerRepository::class)] -class Teamer implements TimestampableEntityInterface +class Teamer implements TimestampableEntityInterface, SoftDeletableEntityInterface { use TimestampableEntity; + use SoftDeletableEntity; public const STATUS_NEW = 'new'; public const STATUS_EXISTING = 'existing'; diff --git a/src/Entity/Traits/SoftDeletableEntity.php b/src/Entity/Traits/SoftDeletableEntity.php index 8cf5f4f..9b5b055 100644 --- a/src/Entity/Traits/SoftDeletableEntity.php +++ b/src/Entity/Traits/SoftDeletableEntity.php @@ -14,7 +14,7 @@ trait SoftDeletableEntity return $this->deletedAt; } - public function setDeletedAt(\DateTimeImmutable $deletedAt): static + public function setDeletedAt(?\DateTimeImmutable $deletedAt): static { $this->deletedAt = $deletedAt; @@ -28,6 +28,13 @@ trait SoftDeletableEntity return $this; } + public function setRestored(): static + { + $this->setDeletedAt(null); + + return $this; + } + public function isDeleted(): bool { return null !== $this->getDeletedAt(); diff --git a/src/Entity/User.php b/src/Entity/User.php index 1cd72c3..d22728a 100644 --- a/src/Entity/User.php +++ b/src/Entity/User.php @@ -2,6 +2,7 @@ namespace App\Entity; +use App\Entity\Traits\SoftDeletableEntity; use App\Entity\Traits\TimestampableEntity; use App\Repository\UserRepository; use Doctrine\DBAL\Types\Types; @@ -12,9 +13,10 @@ use Symfony\Component\Validator\Constraints as Assert; use Symfony\Component\Validator\Context\ExecutionContextInterface; #[ORM\Entity(repositoryClass: UserRepository::class)] -class User implements UserInterface, TimestampableEntityInterface +class User implements UserInterface, TimestampableEntityInterface, SoftDeletableEntityInterface { use TimestampableEntity; + use SoftDeletableEntity; /** * Assignable roles and their labels. @@ -315,6 +317,11 @@ class User implements UserInterface, TimestampableEntityInterface public function setTeamer(?Teamer $teamer): static { + // keep the inverse side in sync: it is only ever hydrated from the database, so + // without this a teamer attached during the request has no user to point back to + $this->teamer?->setUser(null); + $teamer?->setUser($this); + $this->teamer = $teamer; return $this; @@ -408,6 +415,17 @@ class User implements UserInterface, TimestampableEntityInterface return $this->setDisabledAt(new \DateTimeImmutable()); } + /** + * Whether the account is barred from logging in, for either reason. A block and a + * deletion are independent states: a disciplinary block must survive a deletion and + * the restore that follows it, so the two are only ever combined for decisions, + * never merged into one another. + */ + public function isBlocked(): bool + { + return $this->isDeleted() || $this->isDisabled(); + } + public function getDisabledReason(): ?string { return $this->disabledReason; diff --git a/src/EventListener/DeletedUserSubscriber.php b/src/EventListener/DeletedUserSubscriber.php new file mode 100644 index 0000000..f93e6f4 --- /dev/null +++ b/src/EventListener/DeletedUserSubscriber.php @@ -0,0 +1,54 @@ + ['onKernelRequest', 10], + ]; + } + + public function onKernelRequest(RequestEvent $event): void + { + if (false === $event->isMainRequest()) { + return; + } + + $user = $this->security->getUser(); + + if (false === $user instanceof User || false === $user->isDeleted()) { + return; + } + + $this->security->logout(false); + + $event->setResponse(new RedirectResponse($this->urlGenerator->generate('app_security_login'))); + } +} diff --git a/src/EventListener/EmailNotificationSubscriber.php b/src/EventListener/EmailNotificationSubscriber.php index d278f1a..fc55012 100644 --- a/src/EventListener/EmailNotificationSubscriber.php +++ b/src/EventListener/EmailNotificationSubscriber.php @@ -4,6 +4,7 @@ namespace App\EventListener; use App\Email\Mailer; use App\Entity\Application; +use App\Entity\Teamer; use App\Entity\Upload; use App\Event\ApplicationStatusEvent; use App\Event\AssignmentCalledOffEvent; @@ -46,6 +47,13 @@ class EmailNotificationSubscriber implements EventSubscriberInterface { $disposition = $event->getDisposition(); $teamer = $disposition->getTeamer(); + + // checked before the contract is rendered, so no document is produced for a mail + // that is never sent + if (true === $this->isExcluded($teamer)) { + return; + } + $assignment = $disposition->getAssignment(); $destination = $assignment->getDestination(); @@ -78,6 +86,11 @@ class EmailNotificationSubscriber implements EventSubscriberInterface $disposition = $document->getDisposition(); $teamer = $disposition->getTeamer(); + + if (true === $this->isExcluded($teamer)) { + return; + } + $assignment = $disposition->getAssignment(); $destination = $assignment->getDestination(); @@ -152,6 +165,10 @@ class EmailNotificationSubscriber implements EventSubscriberInterface $disposition = $document->getDisposition(); $teamer = $disposition->getTeamer(); + if (true === $this->isExcluded($teamer)) { + return; + } + $documentTypeLabel = match ($document->getType()) { Upload::TYPE_CONTRACT => 'dein Honorarvertrag', Upload::TYPE_INVOICE => 'deine Honorarnote', @@ -182,6 +199,10 @@ class EmailNotificationSubscriber implements EventSubscriberInterface $teamer = $application->getTeamer(); + if (true === $this->isExcluded($teamer)) { + return; + } + $this->mailer->createAndSendEmail([ 'application' => $application, ], [ @@ -203,6 +224,10 @@ class EmailNotificationSubscriber implements EventSubscriberInterface $disposition = $event->getDisposition(); $teamer = $disposition->getTeamer(); + if (true === $this->isExcluded($teamer)) { + return; + } + $this->mailer->createAndSendEmail([ 'assignment' => $disposition->getAssignment(), 'reason' => $disposition->getCalledOffReason(), @@ -222,6 +247,10 @@ class EmailNotificationSubscriber implements EventSubscriberInterface foreach ($assignment->getDispositions() as $disposition) { $teamer = $disposition->getTeamer(); + if (true === $this->isExcluded($teamer)) { + continue; + } + $this->mailer->createAndSendEmail([ 'assignment' => $assignment, ], [ @@ -231,4 +260,13 @@ class EmailNotificationSubscriber implements EventSubscriberInterface ]); } } + + /** + * A deleted teamer is excluded from all communication. Their dispositions and + * documents stay in place and remain visible, they are simply no longer written to. + */ + private function isExcluded(?Teamer $teamer): bool + { + return null === $teamer || true === $teamer->isDeleted(); + } } diff --git a/src/EventListener/RequiredTeamerCheckSubscriber.php b/src/EventListener/RequiredTeamerCheckSubscriber.php index 1988e00..271f371 100644 --- a/src/EventListener/RequiredTeamerCheckSubscriber.php +++ b/src/EventListener/RequiredTeamerCheckSubscriber.php @@ -58,6 +58,9 @@ class RequiredTeamerCheckSubscriber implements EventSubscriberInterface // while a check is outstanding - otherwise a teamer asked to confirm // wrong data has no way to fix it first 'app_teamer_profile_index', + // deleting the own account must stay reachable while a check is outstanding, + // otherwise a teamer could be bounced into a check forever with no way out + 'app_teamer_profile_delete_account', 'app_upload_delete', 'app_security_logout', ], true)) { diff --git a/src/Form/DeleteAccountType.php b/src/Form/DeleteAccountType.php new file mode 100644 index 0000000..0573179 --- /dev/null +++ b/src/Form/DeleteAccountType.php @@ -0,0 +1,26 @@ +add('confirmed', CheckboxType::class, [ + 'label' => 'Ja, ich möchte meinen Account endgültig löschen', + 'mapped' => false, + 'constraints' => [ + new IsTrue([ + 'message' => 'Bitte bestätige die Löschung', + ]), + ], + ]); + } +} diff --git a/src/Form/TeamerFilterType.php b/src/Form/TeamerFilterType.php index 02ee4bf..8ed5bb4 100644 --- a/src/Form/TeamerFilterType.php +++ b/src/Form/TeamerFilterType.php @@ -59,6 +59,10 @@ class TeamerFilterType extends AbstractType 'label' => 'inaktive Teamer:innen anzeigen', 'required' => false, ]) + ->add('includeDeleted', CheckboxType::class, [ + 'label' => 'gelöschte Teamer:innen anzeigen', + 'required' => false, + ]) ->add('apply', SubmitType::class, [ 'label' => 'filtern', ]) diff --git a/src/Model/TeamerFilterDto.php b/src/Model/TeamerFilterDto.php index dea48c5..eb6cbfa 100644 --- a/src/Model/TeamerFilterDto.php +++ b/src/Model/TeamerFilterDto.php @@ -13,6 +13,7 @@ class TeamerFilterDto extends AbstractFilterDto protected bool $driverLicenseVerified = false; protected bool $noTrainings = false; protected bool $includeInactive = false; + protected bool $includeDeleted = false; public function getName(): ?string { @@ -119,4 +120,16 @@ class TeamerFilterDto extends AbstractFilterDto return $this; } + + public function isIncludeDeleted(): bool + { + return $this->includeDeleted; + } + + public function setIncludeDeleted(bool $includeDeleted): static + { + $this->includeDeleted = $includeDeleted; + + return $this; + } } diff --git a/src/Repository/AvailabilityRepository.php b/src/Repository/AvailabilityRepository.php index f32d757..643ab60 100644 --- a/src/Repository/AvailabilityRepository.php +++ b/src/Repository/AvailabilityRepository.php @@ -41,7 +41,12 @@ class AvailabilityRepository extends ServiceEntityRepository if (false === $custom) { $qb->andWhere($qb->expr()->isNull('availability.owner')); } else { - $qb->andWhere($qb->expr()->isNotNull('availability.owner')); + $qb + ->andWhere($qb->expr()->isNotNull('availability.owner')) + // a deleted teamer takes no further assignments, so their own + // availabilities must no longer be offered for disposition + ->andWhere($qb->expr()->isNull('teamer.deletedAt')) + ; } return $qb->getQuery(); @@ -49,6 +54,10 @@ class AvailabilityRepository extends ServiceEntityRepository public function getSelectableForTeamer(Teamer $teamer): array { + if (true === $teamer->isDeleted()) { + return []; + } + $qb = $this->createQueryBuilder('availability'); return $qb diff --git a/src/Repository/TeamerRepository.php b/src/Repository/TeamerRepository.php index a942335..a812b3b 100644 --- a/src/Repository/TeamerRepository.php +++ b/src/Repository/TeamerRepository.php @@ -106,7 +106,14 @@ class TeamerRepository extends ServiceEntityRepository ; } - if (false === $filterDto->isIncludeInactive()) { + if (false === $filterDto->isIncludeDeleted()) { + $qb->andWhere($qb->expr()->isNull('teamer.deletedAt')); + } + + // a deleted teamer never logs in again and would always count as inactive, so + // asking for them has to lift this filter - otherwise they could not be found + // again to be restored + if (false === $filterDto->isIncludeInactive() && false === $filterDto->isIncludeDeleted()) { $qb ->andWhere($qb->expr()->gte('user.lastLoginAt', ':lastLoginThreshold')) ->setParameter('lastLoginThreshold', new \DateTimeImmutable($this->teamerInactivePeriod)) @@ -125,6 +132,7 @@ class TeamerRepository extends ServiceEntityRepository $qb->expr()->like('teamer.lastName', ':search'), $qb->expr()->like('teamer.firstName', ':search'), )) + ->andWhere($qb->expr()->isNull('teamer.deletedAt')) ->orderBy('teamer.lastName', 'ASC') ->addOrderBy('teamer.firstName', 'ASC') ->setParameter('search', '%'.$this->escapeLikeWildcards($search).'%') diff --git a/src/Repository/UserRepository.php b/src/Repository/UserRepository.php index 016f8ac..954f977 100644 --- a/src/Repository/UserRepository.php +++ b/src/Repository/UserRepository.php @@ -52,6 +52,9 @@ class UserRepository extends ServiceEntityRepository )); return $qb + // andWhere after a chain of orWhere yields "(... OR ...) AND ...", so this + // narrows the whole list rather than widening it by one more alternative + ->andWhere($qb->expr()->isNull('u.deletedAt')) ->orderBy('u.lastName', 'ASC') ->getQuery() ->getResult() @@ -82,6 +85,7 @@ class UserRepository extends ServiceEntityRepository return $qb ->andWhere('JSON_CONTAINS(user.roles, :role) = 1') + ->andWhere($qb->expr()->isNull('user.deletedAt')) ->setParameter('role', json_encode($role)) ->getQuery() ->getResult() @@ -98,6 +102,7 @@ class UserRepository extends ServiceEntityRepository $qb->expr()->like('user.firstName', ':search'), )) ->andWhere('JSON_CONTAINS(user.roles, :role) = 1') + ->andWhere($qb->expr()->isNull('user.deletedAt')) ->orderBy('user.lastName', 'ASC') ->addOrderBy('user.firstName', 'ASC') ->setParameter('search', '%'.$this->escapeLikeWildcards($search).'%') diff --git a/src/Security/BpnAuthenticator.php b/src/Security/BpnAuthenticator.php index 985e92f..f1c1975 100644 --- a/src/Security/BpnAuthenticator.php +++ b/src/Security/BpnAuthenticator.php @@ -140,6 +140,19 @@ class BpnAuthenticator extends AbstractLoginFormAuthenticator implements Authent ->findLocalUser($profileResponse) ; + // A deleted account is excluded from every process, and the CRM must not be able + // to undo that: no data is written back, no role is granted or revoked, not even + // lastLoginAt is bumped. It is returned untouched so the UserChecker can refuse + // the login and say why. Only an admin restores it. + if (true === $user?->isDeleted()) { + $this->logger->info('Skip CRM sync for deleted user', [ + 'user_id' => $user->getId(), + 'user_email' => $email, + ]); + + return $user; + } + // BusPro knows this person but grants them nothing in this application, so they // are no user of it: never create an account, block an existing one. Returning // the blocked user lets the UserChecker explain why the login was refused. diff --git a/src/Security/UserChecker.php b/src/Security/UserChecker.php index 35d3f74..bd1c521 100644 --- a/src/Security/UserChecker.php +++ b/src/Security/UserChecker.php @@ -15,6 +15,12 @@ class UserChecker implements UserCheckerInterface return; } + // checked before the block, as a deletion is the stronger statement and its + // message has to win when an account carries both + if (true === $user->isDeleted()) { + throw new CustomUserMessageAccountStatusException('Dein Account wurde gelöscht. Wende dich an das Team, wenn du ihn wiederherstellen möchtest.'); + } + if (true === $user->isDisabled()) { throw new CustomUserMessageAccountStatusException('Dein Account wurde gesperrt: '.$user->getDisabledReason()); } diff --git a/src/Security/Voter/ImpersonationVoter.php b/src/Security/Voter/ImpersonationVoter.php index 103a91f..a67fc95 100644 --- a/src/Security/Voter/ImpersonationVoter.php +++ b/src/Security/Voter/ImpersonationVoter.php @@ -44,6 +44,12 @@ class ImpersonationVoter extends Voter return false; } + // a deleted account is excluded from every process, so impersonating it must not + // become a way back into the teamer area + if (true === $targetUser->isDeleted()) { + return false; + } + // Admin is the only role allowed to impersonate if (false === $this->security->isGranted('ROLE_ADMIN')) { return false; diff --git a/src/Service/Common/TeamerFilterHandler.php b/src/Service/Common/TeamerFilterHandler.php index c3b10f5..23a7861 100644 --- a/src/Service/Common/TeamerFilterHandler.php +++ b/src/Service/Common/TeamerFilterHandler.php @@ -33,6 +33,10 @@ class TeamerFilterHandler extends AbstractFilterHandler $filterDto->setIncludeInactive((bool) $data['include_inactive']); } + if (isset($data['include_deleted'])) { + $filterDto->setIncludeDeleted((bool) $data['include_deleted']); + } + if (0 < count($data['job_profiles'])) { $jobProfiles = $this ->entityManager @@ -57,6 +61,7 @@ class TeamerFilterHandler extends AbstractFilterHandler 'driver_license_verified' => $filterDto->hasDriverLicenseVerified(), 'no_trainings' => $filterDto->hasNoTrainings(), 'include_inactive' => $filterDto->isIncludeInactive(), + 'include_deleted' => $filterDto->isIncludeDeleted(), 'job_profiles' => array_map(function (JobProfile $jobProfile) { return $jobProfile->getId(); }, $filterDto->getJobProfiles()), diff --git a/src/Service/Cron/DispositionReminderService.php b/src/Service/Cron/DispositionReminderService.php index 7b27132..a8dcfd9 100644 --- a/src/Service/Cron/DispositionReminderService.php +++ b/src/Service/Cron/DispositionReminderService.php @@ -30,7 +30,9 @@ class DispositionReminderService ->innerJoin('disposition.assignment', 'assignment') ->innerJoin('assignment.destination', 'destination') ->innerJoin('disposition.documents', 'document', Join::WITH, 'document.type = :documentType') + ->innerJoin('disposition.teamer', 'teamer') ->where($qb->expr()->andX( + $qb->expr()->isNull('teamer.deletedAt'), $qb->expr()->notIn('assignment.status', ':statusAssignment'), $qb->expr()->eq('destination.dateFrom', ':dateFrom'), $qb->expr()->eq('document.status', ':statusContract'), diff --git a/src/Service/Cron/UploadReminderService.php b/src/Service/Cron/UploadReminderService.php index a194c50..9fc95a5 100644 --- a/src/Service/Cron/UploadReminderService.php +++ b/src/Service/Cron/UploadReminderService.php @@ -35,8 +35,12 @@ class UploadReminderService ->innerJoin('disposition.assignment', 'assignment') ->innerJoin('assignment.destination', 'destination') ->leftJoin('disposition.documents', 'document', Join::WITH, 'document.type = :documentType') + ->innerJoin('disposition.teamer', 'teamer') ->where($qb->expr()->andX( $qb->expr()->isNull('document'), + // deleted teamers are excluded here rather than when sending, so that the + // count reported below stays truthful + $qb->expr()->isNull('teamer.deletedAt'), $qb->expr()->orX( $qb->expr()->eq('document.createdAt', ':contractDueDateFirst'), $qb->expr()->eq('document.createdAt', ':contractDueDateSecond'), @@ -90,8 +94,10 @@ class UploadReminderService ->innerJoin('disposition.assignment', 'assignment') ->innerJoin('assignment.destination', 'destination') ->leftJoin('disposition.documents', 'document', Join::WITH, 'document.type = :documentType') + ->innerJoin('disposition.teamer', 'teamer') ->where($qb->expr()->andX( $qb->expr()->isNull('document'), + $qb->expr()->isNull('teamer.deletedAt'), $qb->expr()->orX( // Due date can be determined by assignment's date which potentially overrides destination date $qb->expr()->andX( diff --git a/src/Service/Teamer/AccountDeletionHandler.php b/src/Service/Teamer/AccountDeletionHandler.php new file mode 100644 index 0000000..e04b619 --- /dev/null +++ b/src/Service/Teamer/AccountDeletionHandler.php @@ -0,0 +1,98 @@ +resolvePair($subject); + + if (false === $this->isDeleted($teamer, $user)) { + $teamer?->setDeleted(); + $user?->setDeleted(); + + $this->entityManager->flush(); + + $this->logger->info('Delete account', [...$this->logContext($teamer, $user), 'source' => $source]); + } + } + + public function restore(Teamer|User $subject): void + { + [$teamer, $user] = $this->resolvePair($subject); + + if (true === $this->isDeleted($teamer, $user)) { + $teamer?->setRestored(); + $user?->setRestored(); + + $this->entityManager->flush(); + + $this->logger->info('Restore account', $this->logContext($teamer, $user)); + } + } + + /** + * @return array{0: ?Teamer, 1: ?User} + */ + private function resolvePair(Teamer|User $subject): array + { + if ($subject instanceof Teamer) { + return [$subject, $subject->getUser()]; + } + + return [$subject->getTeamer(), $subject]; + } + + /** + * Either side being flagged counts as deleted, so that a pair left inconsistent by + * an earlier failure is repaired rather than skipped. + */ + private function isDeleted(?Teamer $teamer, ?User $user): bool + { + return true === $teamer?->isDeleted() || true === $user?->isDeleted(); + } + + /** + * @return array + */ + private function logContext(?Teamer $teamer, ?User $user): array + { + return [ + 'teamer' => $teamer?->getFullName(), + 'teamer_id' => $teamer?->getId(), + 'user' => $user?->getEmail(), + 'user_id' => $user?->getId(), + ]; + } +} diff --git a/src/Validator/Constraints/Application.php b/src/Validator/Constraints/Application.php index 6984c0c..10ae123 100644 --- a/src/Validator/Constraints/Application.php +++ b/src/Validator/Constraints/Application.php @@ -9,6 +9,8 @@ class Application extends Constraint { public string $message = 'Diese Bewerbung überlappt sich mit einem deiner bestätigten Einsätze'; + public string $deletedMessage = 'Für einen gelöschten Account können keine Bewerbungen angelegt werden'; + public function getTargets(): array|string { return static::CLASS_CONSTRAINT; diff --git a/src/Validator/Constraints/ApplicationValidator.php b/src/Validator/Constraints/ApplicationValidator.php index 4243e5f..e4a32d1 100644 --- a/src/Validator/Constraints/ApplicationValidator.php +++ b/src/Validator/Constraints/ApplicationValidator.php @@ -21,6 +21,18 @@ class ApplicationValidator extends ConstraintValidator $teamer = $application->getTeamer(); + // a deleted teamer is excluded from every process, so no new application may be + // created for them - not even by an admin acting on their behalf + if (true === $teamer->isDeleted()) { + $this + ->context + ->buildViolation($constraint->deletedMessage) + ->addViolation() + ; + + return; + } + if (true === $teamer->isAllowOverlappingApplications()) { return; } diff --git a/templates/_partials/_disposition_data.html.twig b/templates/_partials/_disposition_data.html.twig index 9d7fca3..a45281c 100644 --- a/templates/_partials/_disposition_data.html.twig +++ b/templates/_partials/_disposition_data.html.twig @@ -1,5 +1,6 @@ {% set assignment = disposition.assignment %} {{ disposition.teamer.fullName(true) }} +{% include '_partials/_teamer_deleted_badge.html.twig' with { 'teamer': disposition.teamer } %} {% include '_partials/_dot.html.twig' %} {{ assignment.effectivePeriod.start|date('d.m.y') }} - {{ assignment.effectivePeriod.end|date('d.m.y') }} {% include '_partials/_dot.html.twig' %} diff --git a/templates/_partials/_teamer_deleted_badge.html.twig b/templates/_partials/_teamer_deleted_badge.html.twig new file mode 100644 index 0000000..221c1a8 --- /dev/null +++ b/templates/_partials/_teamer_deleted_badge.html.twig @@ -0,0 +1,6 @@ +{% if teamer is not null and teamer.deleted %} + + gelöscht + +{% endif %} diff --git a/templates/admin/application/index.html.twig b/templates/admin/application/index.html.twig index 7c0694f..76a78cb 100644 --- a/templates/admin/application/index.html.twig +++ b/templates/admin/application/index.html.twig @@ -100,6 +100,7 @@ {{ application.teamer }} + {% include '_partials/_teamer_deleted_badge.html.twig' with { 'teamer': application.teamer } %}
+ Möchtest du {{ teamer }} und den zugehörigen Benutzeraccount wirklich löschen? +
+
+ Die Person wird aus allen Listen, Formularen und dem Mailversand ausgeschlossen und kann + sich nicht mehr anmelden. Bestehende Einsätze, Dokumente und Feedbacks bleiben erhalten + und weiterhin sichtbar. +
+ {% if pendingApplicationCount > 0 or upcomingDispositionCount > 0 %} +
+
Achtung: offene Vorgänge
+
    + {% if pendingApplicationCount > 0 %} +
  • {{ pendingApplicationCount }} offene Bewerbung(en)
    • + {% endif %} + {% if upcomingDispositionCount > 0 %} +
  • {{ upcomingDispositionCount }} anstehende(r) Einsatz/Einsätze
    • + {% endif %} +
+
+ Diese bleiben bestehen, die Person wird darüber aber nicht mehr benachrichtigt. + Sage sie bei Bedarf vorher ab. +
+
+ {% endif %} +{% endblock %} + +{% block button_confirm %}Löschen{% endblock %} diff --git a/templates/admin/teamer/modal_restore_account.html.twig b/templates/admin/teamer/modal_restore_account.html.twig new file mode 100644 index 0000000..bf457af --- /dev/null +++ b/templates/admin/teamer/modal_restore_account.html.twig @@ -0,0 +1,17 @@ +{% extends 'htmx_confirmation_modal.html.twig' %} + +{% block title %}Account wiederherstellen{% endblock %} + +{% block content %} +
+ Möchtest du {{ teamer }} und den zugehörigen Benutzeraccount wirklich + wiederherstellen? +
+
+ Die Person erscheint danach wieder in allen Listen und erhält wieder E-Mails. Liegt in + BusPro keine Berechtigung mehr vor, wird der Account bei der nächsten Anmeldung erneut + gesperrt. +
+{% endblock %} + +{% block button_confirm %}Wiederherstellen{% endblock %} diff --git a/templates/administrative/assignment/detail.html.twig b/templates/administrative/assignment/detail.html.twig index 18230c5..ee2cdb7 100644 --- a/templates/administrative/assignment/detail.html.twig +++ b/templates/administrative/assignment/detail.html.twig @@ -48,6 +48,7 @@ hx-target="body" hx-swap="beforeend"> {{ application.teamer }} + {% include '_partials/_teamer_deleted_badge.html.twig' with { 'teamer': application.teamer } %} {{ icon('info', 'w-4 h-4') }} @@ -155,6 +156,7 @@ hx-target="body" hx-swap="beforeend"> {{ disposition.teamer }} + {% include '_partials/_teamer_deleted_badge.html.twig' with { 'teamer': disposition.teamer } %} {{ icon('info', 'w-4 h-4') }} diff --git a/templates/administrative/document/index.html.twig b/templates/administrative/document/index.html.twig index 240261b..4fb2e47 100644 --- a/templates/administrative/document/index.html.twig +++ b/templates/administrative/document/index.html.twig @@ -83,6 +83,7 @@ {{ disposition.teamer }} + {% include '_partials/_teamer_deleted_badge.html.twig' with { 'teamer': disposition.teamer } %} {{ upload.status|upload_status_badge }} diff --git a/templates/administrative/feedback/index.html.twig b/templates/administrative/feedback/index.html.twig index 052d0c6..8d9e8e8 100644 --- a/templates/administrative/feedback/index.html.twig +++ b/templates/administrative/feedback/index.html.twig @@ -60,6 +60,7 @@ {{ feedback.teamer.firstName }} + {% include '_partials/_teamer_deleted_badge.html.twig' with { 'teamer': feedback.teamer } %} {% if feedback.assignmentDateFrom and feedback.assignmentDateTo %} diff --git a/templates/administrative/teamer/index.html.twig b/templates/administrative/teamer/index.html.twig index 54545e7..7d4c949 100644 --- a/templates/administrative/teamer/index.html.twig +++ b/templates/administrative/teamer/index.html.twig @@ -104,7 +104,19 @@ {{ icon('check') }} {% endif %} - {% if teamer.user is not null and teamer.user.disabled %} + {% if teamer.deleted %} + {% if is_granted('ROLE_ADMIN') %} + + {% endif %} + {% elseif teamer.user is not null and teamer.user.disabled %} {% if is_granted('ROLE_ADMIN') %} + {% endif %} {% endif %} {{ icon('user') }} + {% if teamer.deleted %} +
+ gel. am {{ teamer.deletedAt | date('d.m.Y') }} +
+ {% endif %} {% if teamer.user is not null and teamer.user.disabled %}
gesp. am {{ teamer.user.disabledAt | date('d.m.Y') }} diff --git a/templates/common/modal_teamer_filter.html.twig b/templates/common/modal_teamer_filter.html.twig index 95dd9f2..c83e881 100644 --- a/templates/common/modal_teamer_filter.html.twig +++ b/templates/common/modal_teamer_filter.html.twig @@ -14,6 +14,7 @@ {{ form_row(filterForm.driverLicenseVerified) }} {{ form_row(filterForm.noTrainings) }} {{ form_row(filterForm.includeInactive) }} + {{ form_row(filterForm.includeDeleted) }}
{{ form_widget(filterForm.apply, { 'attr': { 'class': 'btn' } }) }} diff --git a/templates/teamer/profile/index.html.twig b/templates/teamer/profile/index.html.twig index f3271dd..4886296 100644 --- a/templates/teamer/profile/index.html.twig +++ b/templates/teamer/profile/index.html.twig @@ -164,6 +164,23 @@
+
+

+ Account löschen +

+
+ Du kannst deinen Account löschen. Du wirst danach aus allen Listen + und dem E-Mail-Versand ausgeschlossen und kannst dich nicht mehr + anmelden. +
+ +
diff --git a/templates/teamer/profile/modal_delete_account.html.twig b/templates/teamer/profile/modal_delete_account.html.twig new file mode 100644 index 0000000..b98c1fe --- /dev/null +++ b/templates/teamer/profile/modal_delete_account.html.twig @@ -0,0 +1,27 @@ +{% extends 'htmx_modal.html.twig' %} + +{% block title %}Account löschen{% endblock %} + +{% block content %} + {{ form_start(form) }} +
+
+ Wenn du deinen Account löschst, wirst du aus allen Listen, Formularen und dem + E-Mail-Versand ausgeschlossen und kannst dich nicht mehr anmelden. +
+
+ Deine bisherigen Einsätze, Honorarverträge und Honorarnoten bleiben aus rechtlichen + Gründen gespeichert und für das Team weiterhin sichtbar. +
+
+ Wenn du deinen Account später wieder brauchst, wende dich bitte an das Büro. Du + kannst ihn nicht selbst wiederherstellen. +
+ {{ form_row(form.confirmed) }} +
+ + {{ form_rest(form) }} + {{ form_end(form) }} +{% endblock %} diff --git a/tests/BusProNet/UserDataHandlerTest.php b/tests/BusProNet/UserDataHandlerTest.php index f1f9dcf..08ea8ea 100644 --- a/tests/BusProNet/UserDataHandlerTest.php +++ b/tests/BusProNet/UserDataHandlerTest.php @@ -466,6 +466,73 @@ class UserDataHandlerTest extends TestCase $this->assertSame(2, $user->getBusProPersonId()); } + /** + * A deleted account still has to be matched by email, otherwise the caller takes the + * person for unknown and creates a second account for them - which would resurrect + * them under a new row and defeat the deletion entirely. Nothing is written to it. + */ + public function testFindLocalUserReturnsADeletedEmailMatchWithoutWritingToIt(): void + { + $user = (new User()) + ->setFirstName('First') + ->setLastName('Last') + ->setEmail('new@example.com') + ->setBusProAddressId(1) + ->setBusProPersonId(2) + ; + $user->setDeleted(); + + $profileResponse = $this->createProfileResponse(); + + $repository = $this->createMock(ObjectRepository::class); + $repository + ->expects($this->once()) + ->method('findOneBy') + ->willReturn(null); + + $repository + ->expects($this->once()) + ->method('findBy') + ->with([ + 'email' => 'new@example.com', + ]) + ->willReturn([$user]); + + $this->entityManager + ->expects($this->once()) + ->method('getRepository') + ->with(User::class) + ->willReturn($repository); + + $handler = new UserDataHandler($this->entityManager, $this->logger); + $resolvedUser = $handler->findLocalUser($profileResponse); + + $this->assertSame($user, $resolvedUser); + $this->assertSame(1, $user->getBusProAddressId()); + $this->assertSame(2, $user->getBusProPersonId()); + } + + public function testDisableForRevokedCrmRolesLeavesADeletedAccountAlone(): void + { + $user = (new User()) + ->setFirstName('First') + ->setLastName('Last') + ->setEmail('user@example.com') + ->setRoles(['ROLE_TEAMER']) + ; + $user->setDeleted(); + + $this->entityManager + ->expects($this->never()) + ->method('flush'); + + $handler = new UserDataHandler($this->entityManager, $this->logger); + $handler->disableForRevokedCrmRoles($user); + + $this->assertFalse($user->isDisabled()); + $this->assertSame(['ROLE_TEAMER'], $user->getAssignedRoles()); + } + private function createProfileResponse(?int $addressId = 200, ?int $personId = 100): ProfileResponse { $address = (new BusProAddress()) diff --git a/tests/Entity/Traits/SoftDeletableEntityTest.php b/tests/Entity/Traits/SoftDeletableEntityTest.php new file mode 100644 index 0000000..bfa5a5f --- /dev/null +++ b/tests/Entity/Traits/SoftDeletableEntityTest.php @@ -0,0 +1,55 @@ +createEntity(); + + $this->assertFalse($entity->isDeleted()); + $this->assertNull($entity->getDeletedAt()); + } + + public function testSetDeletedStampsTheTimestamp(): void + { + $entity = $this->createEntity(); + $entity->setDeleted(); + + $this->assertTrue($entity->isDeleted()); + $this->assertNotNull($entity->getDeletedAt()); + } + + public function testSetRestoredClearsTheTimestamp(): void + { + $entity = $this->createEntity(); + $entity->setDeleted(); + $entity->setRestored(); + + $this->assertFalse($entity->isDeleted()); + $this->assertNull($entity->getDeletedAt()); + } + + public function testDeletedAtAcceptsNullSoDeletionsCanBeUndone(): void + { + $entity = $this->createEntity(); + $entity->setDeletedAt(new \DateTimeImmutable()); + $entity->setDeletedAt(null); + + $this->assertFalse($entity->isDeleted()); + } + + private function createEntity(): SoftDeletableEntityInterface + { + return new class implements SoftDeletableEntityInterface { + use SoftDeletableEntity; + }; + } +} diff --git a/tests/Entity/UserTest.php b/tests/Entity/UserTest.php index 14a8bd6..7fc554f 100644 --- a/tests/Entity/UserTest.php +++ b/tests/Entity/UserTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace App\Tests\Entity; +use App\Entity\Teamer; use App\Entity\User; use PHPUnit\Framework\TestCase; use Symfony\Component\Validator\ConstraintViolationListInterface; @@ -118,6 +119,44 @@ class UserTest extends TestCase $this->assertNull($enabled->getDisabledAt()); } + /** + * A block and a deletion are independent states: setting one must never imply or + * clear the other, so that a disciplinary block survives a deletion and the restore + * that follows it. + */ + public function testDeletionAndBlockAreIndependentStates(): void + { + $user = new User(); + $this->assertFalse($user->isBlocked()); + + $user->setDeleted(); + $this->assertTrue($user->isBlocked()); + $this->assertFalse($user->isDisabled()); + + $user->setDisabled(true); + $disabledAt = $user->getDisabledAt(); + + $user->setRestored(); + $this->assertFalse($user->isDeleted()); + $this->assertTrue($user->isBlocked()); + $this->assertSame($disabledAt, $user->getDisabledAt()); + + $user->setDisabled(false); + $this->assertFalse($user->isBlocked()); + } + + public function testSettingTheTeamerKeepsTheInverseSideInSync(): void + { + $teamer = new Teamer(); + $user = (new User())->setTeamer($teamer); + + $this->assertSame($user, $teamer->getUser()); + + $user->setTeamer(null); + + $this->assertNull($teamer->getUser()); + } + private function validate(User $user): ConstraintViolationListInterface { return Validation::createValidatorBuilder() diff --git a/tests/EventListener/EmailNotificationSubscriberTest.php b/tests/EventListener/EmailNotificationSubscriberTest.php new file mode 100644 index 0000000..173c7b3 --- /dev/null +++ b/tests/EventListener/EmailNotificationSubscriberTest.php @@ -0,0 +1,107 @@ +mailer = $this->createMock(Mailer::class); + + $this->subscriber = new EmailNotificationSubscriber( + $this->mailer, + $this->createMock(UserRepository::class), + $this->createMock(ContractRenderer::class), + ); + } + + public function testDeletedTeamerIsNotNotifiedAboutCalledOffDisposition(): void + { + $disposition = $this->createDisposition($this->createDeletedTeamer()); + + $this->mailer->expects($this->never())->method('createAndSendEmail'); + + $this->subscriber->onDispositionCalledOff(new DispositionCalledOffEvent($disposition, true)); + } + + public function testActiveTeamerIsNotifiedAboutCalledOffDisposition(): void + { + $disposition = $this->createDisposition($this->createTeamer()); + + $this->mailer->expects($this->once())->method('createAndSendEmail'); + + $this->subscriber->onDispositionCalledOff(new DispositionCalledOffEvent($disposition, true)); + } + + public function testDeletedTeamerIsNotNotifiedAboutRejectedApplication(): void + { + $application = new Application(new Assignment(), $this->createDeletedTeamer()); + $application->setStatus(Application::STATUS_REJECTED); + + $this->mailer->expects($this->never())->method('createAndSendEmail'); + + $this->subscriber->onApplicationStatus( + new ApplicationStatusEvent(new ApplicationStatusDto($application)) + ); + } + + /** + * The call-off of a whole assignment mails every teamer on it, so the exclusion has + * to happen per teamer rather than for the event as a whole. + */ + public function testCalledOffAssignmentSkipsOnlyTheDeletedTeamer(): void + { + $assignment = new Assignment(); + $assignment->addDisposition($this->createDisposition($this->createTeamer(), $assignment)); + $assignment->addDisposition($this->createDisposition($this->createDeletedTeamer(), $assignment)); + + $this->mailer->expects($this->once())->method('createAndSendEmail'); + + $this->subscriber->onAssignmentCalledOff(new AssignmentCalledOffEvent($assignment)); + } + + private function createDisposition(Teamer $teamer, ?Assignment $assignment = null): Disposition + { + return new Disposition(new Application($assignment ?? new Assignment(), $teamer)); + } + + private function createTeamer(): Teamer + { + return (new Teamer())->setCommunication( + (new Communication())->setEmail('teamer@example.com') + ); + } + + private function createDeletedTeamer(): Teamer + { + $teamer = $this->createTeamer(); + $teamer->setDeleted(); + + return $teamer; + } +} diff --git a/tests/Security/BpnAuthenticatorTest.php b/tests/Security/BpnAuthenticatorTest.php index 8eec930..4d83269 100644 --- a/tests/Security/BpnAuthenticatorTest.php +++ b/tests/Security/BpnAuthenticatorTest.php @@ -86,6 +86,44 @@ class BpnAuthenticatorTest extends TestCase $this->loadUser(); } + /** + * The CRM must not be able to undo a deletion, in either direction: neither by + * refreshing the account's data nor by blocking it further. + */ + public function testDeletedUserIsReturnedWithoutAnyCrmSync(): void + { + $user = (new User())->setRoles(['ROLE_TEAMER']); + $user->setDeleted(); + + $this->stubApiClient($this->createCrmAttributes()); + + $this->userDataHandler->method('collectRoles')->willReturn(['ROLE_TEAMER']); + $this->userDataHandler->method('findLocalUser')->willReturn($user); + + $this->userDataHandler->expects($this->never())->method('updateLocalUser'); + $this->userDataHandler->expects($this->never())->method('createLocalUser'); + $this->userDataHandler->expects($this->never())->method('disableForRevokedCrmRoles'); + + // returned rather than refused, so the UserChecker can explain the deletion + $this->assertSame($user, $this->loadUser()); + } + + public function testDeletedUserIsNotBlockedWhenTheCrmRevokedEveryRole(): void + { + $user = (new User())->setRoles(['ROLE_TEAMER']); + $user->setDeleted(); + + $this->stubApiClient($this->createCrmAttributes()); + + $this->userDataHandler->method('collectRoles')->willReturn([]); + $this->userDataHandler->method('findLocalUser')->willReturn($user); + + $this->userDataHandler->expects($this->never())->method('disableForRevokedCrmRoles'); + + $this->assertSame($user, $this->loadUser()); + $this->assertFalse($user->isDisabled()); + } + private function createCrmAttributes(): CrmAttributesResponse { return (new CrmAttributesResponse())->setAttributeGroups([new CrmAttributeGroup()]); diff --git a/tests/Security/UserCheckerTest.php b/tests/Security/UserCheckerTest.php new file mode 100644 index 0000000..3206882 --- /dev/null +++ b/tests/Security/UserCheckerTest.php @@ -0,0 +1,69 @@ +userChecker = new UserChecker(); + } + + public function testDeletedUserIsRefused(): void + { + $user = (new User())->setRoles(['ROLE_TEAMER']); + $user->setDeleted(); + + $this->expectException(CustomUserMessageAccountStatusException::class); + $this->expectExceptionMessageMatches('/gelöscht/'); + + $this->userChecker->checkPreAuth($user); + } + + /** + * A deletion is the stronger statement, so its message has to win over the block + * message when an account carries both. + */ + public function testDeletedAndDisabledUserIsRefusedWithTheDeletionMessage(): void + { + $user = (new User())->setRoles(['ROLE_TEAMER']); + $user->setDisabled(true); + $user->setDisabledReason('Disziplinarisch gesperrt'); + $user->setDeleted(); + + $this->expectException(CustomUserMessageAccountStatusException::class); + $this->expectExceptionMessageMatches('/gelöscht/'); + + $this->userChecker->checkPreAuth($user); + } + + public function testDisabledUserStillGetsTheBlockMessage(): void + { + $user = (new User())->setRoles(['ROLE_TEAMER']); + $user->setDisabled(true); + $user->setDisabledReason('Disziplinarisch gesperrt'); + + $this->expectException(CustomUserMessageAccountStatusException::class); + $this->expectExceptionMessage('Dein Account wurde gesperrt: Disziplinarisch gesperrt'); + + $this->userChecker->checkPreAuth($user); + } + + public function testActiveUserWithValidRolePasses(): void + { + $user = (new User())->setRoles(['ROLE_TEAMER']); + + $this->userChecker->checkPreAuth($user); + + $this->assertFalse($user->isBlocked()); + } +} diff --git a/tests/Service/Teamer/AccountDeletionHandlerTest.php b/tests/Service/Teamer/AccountDeletionHandlerTest.php new file mode 100644 index 0000000..9370e96 --- /dev/null +++ b/tests/Service/Teamer/AccountDeletionHandlerTest.php @@ -0,0 +1,148 @@ +entityManager = $this->createMock(EntityManagerInterface::class); + + $this->handler = new AccountDeletionHandler( + $this->entityManager, + $this->createMock(LoggerInterface::class), + ); + } + + public function testDeleteFromTeamerSideFlagsBothEntities(): void + { + $teamer = new Teamer(); + $user = (new User())->setTeamer($teamer); + + $this->entityManager->expects($this->once())->method('flush'); + + $this->handler->delete($teamer, AccountDeletionHandler::SOURCE_ADMIN); + + $this->assertTrue($teamer->isDeleted()); + $this->assertTrue($user->isDeleted()); + } + + public function testDeleteFromUserSideFlagsBothEntities(): void + { + $teamer = new Teamer(); + $user = (new User())->setTeamer($teamer); + + $this->handler->delete($user, AccountDeletionHandler::SOURCE_SELF); + + $this->assertTrue($teamer->isDeleted()); + $this->assertTrue($user->isDeleted()); + } + + public function testDeleteWorksForUserWithoutTeamer(): void + { + $user = new User(); + + $this->entityManager->expects($this->once())->method('flush'); + + $this->handler->delete($user, AccountDeletionHandler::SOURCE_ADMIN); + + $this->assertTrue($user->isDeleted()); + } + + public function testDeleteWorksForTeamerWithoutUser(): void + { + $teamer = new Teamer(); + + $this->entityManager->expects($this->once())->method('flush'); + + $this->handler->delete($teamer, AccountDeletionHandler::SOURCE_ADMIN); + + $this->assertTrue($teamer->isDeleted()); + } + + public function testDeletingTwiceIsANoOpAndKeepsTheOriginalTimestamp(): void + { + $teamer = new Teamer(); + $user = (new User())->setTeamer($teamer); + + $this->entityManager->expects($this->once())->method('flush'); + + $this->handler->delete($teamer, AccountDeletionHandler::SOURCE_ADMIN); + $deletedAt = $teamer->getDeletedAt(); + + $this->handler->delete($teamer, AccountDeletionHandler::SOURCE_ADMIN); + + $this->assertSame($deletedAt, $teamer->getDeletedAt()); + } + + /** + * A deletion is not a block: the two states are independent, so that a disciplinary + * block survives a deletion and the restore that follows it. + */ + public function testDeleteLeavesRolesAndTheBlockUntouched(): void + { + $teamer = new Teamer(); + $user = (new User()) + ->setTeamer($teamer) + ->setRoles(['ROLE_TEAMER', 'ROLE_ADMIN']) + ; + $user->setDisabled(true); + $disabledAt = $user->getDisabledAt(); + + $this->handler->delete($user, AccountDeletionHandler::SOURCE_ADMIN); + + $this->assertSame(['ROLE_TEAMER', 'ROLE_ADMIN'], $user->getAssignedRoles()); + $this->assertSame($disabledAt, $user->getDisabledAt()); + } + + public function testRestoreClearsBothFlags(): void + { + $teamer = new Teamer(); + $user = (new User())->setTeamer($teamer); + + $this->handler->delete($teamer, AccountDeletionHandler::SOURCE_ADMIN); + $this->handler->restore($teamer); + + $this->assertFalse($teamer->isDeleted()); + $this->assertFalse($user->isDeleted()); + } + + public function testRestoreOfAnActiveAccountIsANoOp(): void + { + $teamer = new Teamer(); + + $this->entityManager->expects($this->never())->method('flush'); + + $this->handler->restore($teamer); + + $this->assertFalse($teamer->isDeleted()); + } + + /** + * Should a pair ever be left half-flagged, restoring has to repair it rather than + * skip it because one side already looks active. + */ + public function testRestoreRepairsAnInconsistentPair(): void + { + $teamer = (new Teamer())->setDeleted(); + $user = (new User())->setTeamer($teamer); + + $this->handler->restore($user); + + $this->assertFalse($teamer->isDeleted()); + $this->assertFalse($user->isDeleted()); + } +}