diff --git a/module/Report/src/Listener/DatabaseUpdateListener.php b/module/Report/src/Listener/DatabaseUpdateListener.php index 01e9e4a64..cc9eadea8 100644 --- a/module/Report/src/Listener/DatabaseUpdateListener.php +++ b/module/Report/src/Listener/DatabaseUpdateListener.php @@ -13,22 +13,10 @@ use Database\Model\SubDecision as DatabaseSubDecisionModel; use Doctrine\ORM\EntityManager; use Doctrine\ORM\Event\LifecycleEventArgs; -use Report\Model\SubDecision as SubDecisionModel; -use Report\Model\SubDecision\Abrogation as ReportAbrogationModel; -use Report\Model\SubDecision\Board\Discharge as ReportBoardDischargeModel; -use Report\Model\SubDecision\Board\Installation as ReportBoardInstallationModel; -use Report\Model\SubDecision\Board\Release as ReportBoardReleaseModel; -use Report\Model\SubDecision\Discharge as ReportDischargeModel; -use Report\Model\SubDecision\Foundation as ReportFoundationModel; -use Report\Model\SubDecision\Installation as ReportInstallationModel; -use Report\Model\SubDecision\Key\Granting as ReportKeyGrantingModel; -use Report\Model\SubDecision\Key\Withdrawal as ReportKeyWithdrawalModel; -use Report\Service\Board as BoardService; -use Report\Service\Keyholder as KeyholderService; use Report\Service\Meeting as MeetingService; use Report\Service\Member as MemberService; use Report\Service\Misc as MiscService; -use Report\Service\Organ as OrganService; +use Report\Service\SubDecision as SubDecisionService; /** * Doctrine event listener intended to automatically update reportdb. @@ -38,12 +26,10 @@ class DatabaseUpdateListener protected static bool $isflushing = false; public function __construct( - private readonly BoardService $boardService, - private readonly KeyholderService $keyholderService, private readonly MeetingService $meetingService, private readonly MemberService $memberService, private readonly MiscService $miscService, - private readonly OrganService $organService, + private readonly SubDecisionService $subDecisionService, private readonly EntityManager $emReport, ) { } @@ -87,9 +73,7 @@ public function postUpdate(LifecycleEventArgs $eventArgs): void case $entity instanceof DatabaseSubDecisionModel: $subdecision = $this->meetingService->generateSubDecision($entity); - $this->processBoardMemberUpdates($subdecision); - $this->processKeyholderUpdates($subdecision); - $this->processOrganUpdates($subdecision); + $this->subDecisionService->generateRelated($subdecision); $this->emReport->persist($subdecision); break; @@ -116,55 +100,4 @@ public function postUpdate(LifecycleEventArgs $eventArgs): void $em->flush(); }); } - - public function processOrganUpdates(SubDecisionModel $entity): void - { - switch (true) { - case $entity instanceof ReportFoundationModel: - $this->organService->generateFoundation($entity); - break; - - case $entity instanceof ReportAbrogationModel: - $this->organService->generateAbrogation($entity); - break; - - case $entity instanceof ReportInstallationModel: - $this->organService->generateInstallation($entity); - break; - - case $entity instanceof ReportDischargeModel: - $this->organService->generateDischarge($entity); - break; - } - } - - public function processKeyholderUpdates(SubDecisionModel $entity): void - { - switch (true) { - case $entity instanceof ReportKeyGrantingModel: - $this->keyholderService->generateGranting($entity); - break; - - case $entity instanceof ReportKeyWithdrawalModel: - $this->keyholderService->generateWithdrawal($entity); - break; - } - } - - public function processBoardMemberUpdates(SubDecisionModel $entity): void - { - switch (true) { - case $entity instanceof ReportBoardInstallationModel: - $this->boardService->generateInstallation($entity); - break; - - case $entity instanceof ReportBoardReleaseModel: - $this->boardService->generateRelease($entity); - break; - - case $entity instanceof ReportBoardDischargeModel: - $this->boardService->generateDischarge($entity); - break; - } - } } diff --git a/module/Report/src/Listener/Factory/DatabaseUpdateListenerFactory.php b/module/Report/src/Listener/Factory/DatabaseUpdateListenerFactory.php index a77264a98..434c97d47 100644 --- a/module/Report/src/Listener/Factory/DatabaseUpdateListenerFactory.php +++ b/module/Report/src/Listener/Factory/DatabaseUpdateListenerFactory.php @@ -8,12 +8,10 @@ use Laminas\ServiceManager\Factory\FactoryInterface; use Psr\Container\ContainerInterface; use Report\Listener\DatabaseUpdateListener; -use Report\Service\Board as BoardService; -use Report\Service\Keyholder as KeyholderService; use Report\Service\Meeting as MeetingService; use Report\Service\Member as MemberService; use Report\Service\Misc as MiscService; -use Report\Service\Organ as OrganService; +use Report\Service\SubDecision as SubDecisionService; class DatabaseUpdateListenerFactory implements FactoryInterface { @@ -25,28 +23,22 @@ public function __invoke( $requestedName, ?array $options = null, ): DatabaseUpdateListener { - /** @var BoardService $boardService */ - $boardService = $container->get(BoardService::class); - /** @var KeyholderService $keyholderService */ - $keyholderService = $container->get(KeyholderService::class); /** @var MeetingService $meetingService */ $meetingService = $container->get(MeetingService::class); /** @var MemberService $memberService */ $memberService = $container->get(MemberService::class); /** @var MiscService $miscService */ $miscService = $container->get(MiscService::class); - /** @var OrganService $organService */ - $organService = $container->get(OrganService::class); + /** @var SubDecisionService $subDecisionService */ + $subDecisionService = $container->get(SubDecisionService::class); /** @var EntityManager $emReport */ $emReport = $container->get('doctrine.entitymanager.orm_report'); return new DatabaseUpdateListener( - $boardService, - $keyholderService, $meetingService, $memberService, $miscService, - $organService, + $subDecisionService, $emReport, ); } diff --git a/module/Report/src/Model/Meeting.php b/module/Report/src/Model/Meeting.php index 5845054ab..465fbce24 100644 --- a/module/Report/src/Model/Meeting.php +++ b/module/Report/src/Model/Meeting.php @@ -52,6 +52,7 @@ enumType: MeetingTypes::class, #[OneToMany( targetEntity: Decision::class, mappedBy: 'meeting', + cascade: ['persist', 'remove'], )] private Collection $decisions; diff --git a/module/Report/src/Model/Organ.php b/module/Report/src/Model/Organ.php index d6a607a35..76ac95df1 100644 --- a/module/Report/src/Model/Organ.php +++ b/module/Report/src/Model/Organ.php @@ -126,37 +126,31 @@ enumType: OrganTypes::class, name: 'organ_id', referencedColumnName: 'id', nullable: false, - onDelete: 'CASCADE', )] #[InverseJoinColumn( name: 'meeting_type', referencedColumnName: 'meeting_type', nullable: false, - onDelete: 'CASCADE', )] #[InverseJoinColumn( name: 'meeting_number', referencedColumnName: 'meeting_number', nullable: false, - onDelete: 'CASCADE', )] #[InverseJoinColumn( name: 'decision_point', referencedColumnName: 'decision_point', nullable: false, - onDelete: 'CASCADE', )] #[InverseJoinColumn( name: 'decision_number', referencedColumnName: 'decision_number', nullable: false, - onDelete: 'CASCADE', )] #[InverseJoinColumn( name: 'subdecision_sequence', referencedColumnName: 'sequence', nullable: false, - onDelete: 'CASCADE', )] private Collection $subdecisions; diff --git a/module/Report/src/Module.php b/module/Report/src/Module.php index f37596c4b..510e8c39e 100644 --- a/module/Report/src/Module.php +++ b/module/Report/src/Module.php @@ -21,11 +21,13 @@ use Report\Service\Factory\MemberFactory as MemberServiceFactory; use Report\Service\Factory\MiscFactory as MiscServiceFactory; use Report\Service\Factory\OrganFactory as OrganServiceFactory; +use Report\Service\Factory\SubDecisionFactory as SubDecisionServiceFactory; use Report\Service\Keyholder as KeyholderService; use Report\Service\Meeting as MeetingService; use Report\Service\Member as MemberService; use Report\Service\Misc as MiscService; use Report\Service\Organ as OrganService; +use Report\Service\SubDecision as SubDecisionService; class Module { @@ -53,6 +55,7 @@ public function getServiceConfig(): array MemberMapper::class => MemberMapperFactory::class, MiscService::class => MiscServiceFactory::class, OrganService::class => OrganServiceFactory::class, + SubDecisionService::class => SubDecisionServiceFactory::class, DatabaseDeletionListener::class => DatabaseDeletionListenerFactory::class, DatabaseUpdateListener::class => DatabaseUpdateListenerFactory::class, ], diff --git a/module/Report/src/Service/Factory/MeetingFactory.php b/module/Report/src/Service/Factory/MeetingFactory.php index 080c9ca5e..b268fd015 100644 --- a/module/Report/src/Service/Factory/MeetingFactory.php +++ b/module/Report/src/Service/Factory/MeetingFactory.php @@ -10,6 +10,7 @@ use Laminas\ServiceManager\Factory\FactoryInterface; use Psr\Container\ContainerInterface; use Report\Service\Meeting as MeetingService; +use Report\Service\SubDecision as SubDecisionService; class MeetingFactory implements FactoryInterface { @@ -23,6 +24,7 @@ public function __invoke( ): MeetingService { $translator = $container->get(MvcTranslator::class); $meetingMapper = $container->get(MeetingMapper::class); + $subDecisionService = $container->get(SubDecisionService::class); /** @var EntityManager $emReport */ $emReport = $container->get('doctrine.entitymanager.orm_report'); /** @var array $config */ @@ -32,6 +34,7 @@ public function __invoke( return new MeetingService( $translator, $meetingMapper, + $subDecisionService, $emReport, $config, $mailTransport, diff --git a/module/Report/src/Service/Factory/SubDecisionFactory.php b/module/Report/src/Service/Factory/SubDecisionFactory.php new file mode 100644 index 000000000..626b6acf2 --- /dev/null +++ b/module/Report/src/Service/Factory/SubDecisionFactory.php @@ -0,0 +1,41 @@ +get(BoardService::class); + /** @var KeyholderService $keyholderService */ + $keyholderService = $container->get(KeyholderService::class); + /** @var OrganService $organService */ + $organService = $container->get(OrganService::class); + /** @var EntityManager $emReport */ + $emReport = $container->get('doctrine.entitymanager.orm_report'); + + return new SubDecisionService( + $emReport, + $boardService, + $keyholderService, + $organService, + ); + } +} diff --git a/module/Report/src/Service/Meeting.php b/module/Report/src/Service/Meeting.php index 3fa8fbfd5..abff63256 100644 --- a/module/Report/src/Service/Meeting.php +++ b/module/Report/src/Service/Meeting.php @@ -11,7 +11,6 @@ use Database\Model\Member as DatabaseMemberModel; use Database\Model\SubDecision as DatabaseSubDecisionModel; use Doctrine\ORM\EntityManager; -use Exception; use Laminas\Mail\Header\MessageId; use Laminas\Mail\Message; use Laminas\Mail\Transport\TransportInterface; @@ -23,6 +22,8 @@ use Report\Model\Meeting as ReportMeetingModel; use Report\Model\Member as ReportMemberModel; use Report\Model\SubDecision as ReportSubDecisionModel; +use Report\Service\SubDecision as SubDecisionService; +use RuntimeException; use Throwable; use function array_reverse; @@ -38,6 +39,7 @@ class Meeting public function __construct( private readonly Translator $translator, private readonly MeetingMapper $meetingMapper, + private readonly SubDecisionService $subDecisionService, private readonly EntityManager $emReport, private readonly array $config, private readonly TransportInterface $mailTransport, @@ -78,12 +80,11 @@ public function generateMeeting(DatabaseMeetingModel $meeting): void if (null === $reportMeeting) { $reportMeeting = new ReportMeetingModel(); + $reportMeeting->setType($meeting->getType()); + $reportMeeting->setNumber($meeting->getNumber()); + $reportMeeting->setDate($meeting->getDate()); } - $reportMeeting->setType($meeting->getType()); - $reportMeeting->setNumber($meeting->getNumber()); - $reportMeeting->setDate($meeting->getDate()); - foreach ($meeting->getDecisions() as $decision) { try { $this->generateDecision($decision, $reportMeeting); @@ -125,10 +126,10 @@ public function generateDecision( if (null === $reportDecision) { $reportDecision = new ReportDecisionModel(); $reportDecision->setMeeting($reportMeeting); + $reportDecision->setPoint($decision->getPoint()); + $reportDecision->setNumber($decision->getNumber()); } - $reportDecision->setPoint($decision->getPoint()); - $reportDecision->setNumber($decision->getNumber()); $contentNL = []; $contentEN = []; @@ -322,6 +323,10 @@ public function generateSubDecision( ]); $reportSubDecision->setTarget($target); + + // Annulment must be handled here, because it cannot be part of the process{X}Updates because the + // subdecision is the annulment, not the target subdecision(s). + $this->annulDecision($target); } // Abolish decisions are handled by foundationreference @@ -335,6 +340,162 @@ public function generateSubDecision( return $reportSubDecision; } + /** + * Annuls a previously recorded decision and its subdecisions in GEWISDB. + * + * This function reverts the effects of a target decision by undoing or removing its associated subdecisions. Each + * subdecision type is handled explicitly to ensure that the data remains consistent and auditable. + * + * GEWISDB operates as a ledger, meaning the chronological order of decisions must be preserved. A target decision + * made at point X may be annulled at point Z, but any relevant decisions that influence the target (at points Y) + * must lie strictly between X and Z. Annulments cannot be applied retroactively or out of sequence. Violating this + * breaks the ledger assumption and will result in an inconsistent and potentially irrecoverable state. + * + * This ordering is what allows us to perform the annulment at point Z, because at that time all points Y will be + * known and processed. + * + * NOTE: to adhere to our ordering assumption within a decision, we must loop through its subdecisions in reverse. + */ + private function annulDecision(ReportDecisionModel $target): void + { + foreach (array_reverse($target->getSubDecisions()->toArray()) as $targetSubDecision) { + if ($targetSubDecision instanceof ReportSubDecisionModel\Installation) { + // installation + $organMember = $targetSubDecision->getOrganMember(); + + // Cannot annul if organ membership changed since installation. + if ( + null !== $organMember->getDischargeDate() + || !$organMember->getInstallation()->getReappointments()->isEmpty() + ) { + throw new RuntimeException( + 'Cannot annul installation due to other relevant decisions after installation', + ); + } + + $targetSubDecision->getFoundation()->getOrgan()->getMembers()->removeElement($organMember); + $this->emReport->remove($organMember); + } elseif ($targetSubDecision instanceof ReportSubDecisionModel\Discharge) { + // discharge + $organMember = $targetSubDecision->getInstallation()->getOrganMember(); + $organMember->setDischargeDate(null); + } elseif ($targetSubDecision instanceof ReportSubDecisionModel\Reappointment) { + // reappointment + $installation = $targetSubDecision->getInstallation(); + + // Cannot annul if the installation has already been discharged. + if (null !== $installation->getDischarge()) { + throw new RuntimeException('Cannot annul reappointment due to discharge after reappointment'); + } + + // Cannot annul if there are later reappointments tied to the same installation. + foreach ($installation->getReappointments() as $otherReappointment) { + if ($otherReappointment === $targetSubDecision) { + continue; + } + + // Compare ordering: if another reappointment comes after this one, annulment is invalid. + if ($this->isAfter($otherReappointment, $targetSubDecision)) { + throw new RuntimeException( + 'Cannot annul reappointment due to other relevant decisions after reappointment', + ); + } + } + + $installation->removeReappointment($targetSubDecision); + } elseif ($targetSubDecision instanceof ReportSubDecisionModel\Foundation) { + // foundation + $organ = $targetSubDecision->getOrgan(); + + // Cannot annul if the organ has installations. + if (!$organ->getMembers()->isEmpty()) { + throw new RuntimeException('Cannot annul foundation due to existing installations in the organ'); + } + + $this->emReport->remove($organ); + } elseif ($targetSubDecision instanceof ReportSubDecisionModel\Abrogation) { + // abrogation + $organ = $targetSubDecision->getFoundation()->getOrgan(); + $organ->setAbrogationDate(null); + } elseif ($targetSubDecision instanceof ReportSubDecisionModel\Board\Installation) { + // board installation + $boardMember = $targetSubDecision->getBoardMember(); + + // Cannot annul if the board member has already been released or discharged. + if ( + null !== $boardMember->getReleaseDate() + || null !== $boardMember->getDischargeDate() + ) { + throw new RuntimeException('Cannot annul board installation due to later release or discharge'); + } + + $this->emReport->remove($boardMember); + } elseif ($targetSubDecision instanceof ReportSubDecisionModel\Board\Release) { + // board release + $installation = $targetSubDecision->getInstallation(); + $boardMember = $installation->getBoardMember(); + + // Cannot annul release if the board member was also discharged afterwards. + if (null !== $boardMember->getDischargeDate()) { + throw new RuntimeException('Cannot annul board release due to later discharge'); + } + + $boardMember->setReleaseDate(null); + } elseif ($targetSubDecision instanceof ReportSubDecisionModel\Board\Discharge) { + // board discharge + $installation = $targetSubDecision->getInstallation(); + $boardMember = $installation->getBoardMember(); + + $boardMember->setDischargeDate(null); + } elseif ($targetSubDecision instanceof ReportSubDecisionModel\Key\Granting) { + // key code granting + $keyholder = $targetSubDecision->getKeyholder(); + + // Cannot annul granting if it has already been withdrawn. + if (null !== $keyholder->getWithdrawnDate()) { + throw new RuntimeException('Cannot annul key granting due to later withdrawal'); + } + + $this->emReport->remove($keyholder); + } elseif ($targetSubDecision instanceof ReportSubDecisionModel\Key\Withdrawal) { + // key code withdrawal + $keyholder = $targetSubDecision->getGranting()->getKeyholder(); + $keyholder->setWithdrawnDate(null); + } elseif ($targetSubDecision instanceof ReportSubDecisionModel\Annulment) { + // This is undefined behaviour. + throw new LogicException('Annulment of a previous annulment is undefined'); + } + + $this->emReport->persist($targetSubDecision); + } + } + + /** + * Determine if $a occurs after $b in the ledger ordering. + */ + private function isAfter( + ReportSubDecisionModel $a, + ReportSubDecisionModel $b, + ): bool { + if ($a->getMeetingType() !== $b->getMeetingType()) { + throw new LogicException('Cannot compare decisions across different meeting types'); + } + + if ($a->getMeetingNumber() !== $b->getMeetingNumber()) { + return $a->getMeetingNumber() > $b->getMeetingNumber(); + } + + if ($a->getDecisionPoint() !== $b->getDecisionPoint()) { + return $a->getDecisionPoint() > $b->getDecisionPoint(); + } + + if ($a->getDecisionNumber() !== $b->getDecisionNumber()) { + return $a->getDecisionNumber() > $b->getDecisionNumber(); + } + + return $a->getSequence() > $b->getSequence(); + } + public function deleteDecision(DatabaseDecisionModel $decision): void { $reportDecision = $this->emReport->getRepository(ReportDecisionModel::class)->find([ @@ -355,8 +516,12 @@ public function deleteSubDecision(ReportSubDecisionModel $subDecision): void { switch (true) { case $subDecision instanceof ReportSubDecisionModel\Annulment: - throw new Exception('Deletion of annulling decisions not implemented'); + $targetDecision = $subDecision->getTarget(); + foreach ($targetDecision->getSubdecisions() as $targetSubDecision) { + $this->subDecisionService->generateRelated($targetSubDecision); + } + break; case $subDecision instanceof ReportSubDecisionModel\Reappointment: $installation = $subDecision->getInstallation(); $installation->removeReappointment($subDecision); @@ -421,11 +586,10 @@ public function deleteSubDecision(ReportSubDecisionModel $subDecision): void * * @psalm-ignore-nullable-return */ - public function findMember(DatabaseMemberModel $member): ReportMemberModel + public function findMember(DatabaseMemberModel $member): ?ReportMemberModel { - $repo = $this->emReport->getRepository(ReportMemberModel::class); - - return $repo->find($member->getLidnr()); + return $this->emReport->getRepository(ReportMemberModel::class) + ->find($member->getLidnr()); } /** diff --git a/module/Report/src/Service/Organ.php b/module/Report/src/Service/Organ.php index 746a7a0dd..6ca8503ac 100644 --- a/module/Report/src/Service/Organ.php +++ b/module/Report/src/Service/Organ.php @@ -10,7 +10,7 @@ use LogicException; use ReflectionProperty; use Report\Model\Organ as ReportOrganModel; -use Report\Model\OrganMember; +use Report\Model\OrganMember as ReportOrganMemberModel; use Report\Model\SubDecision\Abrogation as ReportAbrogationModel; use Report\Model\SubDecision\Discharge as ReportDischargeModel; use Report\Model\SubDecision\Foundation as ReportFoundationModel; @@ -175,7 +175,7 @@ public function generateInstallation(ReportInstallationModel $ref): void } if (null === $organMember) { - $organMember = new OrganMember(); + $organMember = new ReportOrganMemberModel(); // set the ID stuff $organMember->setOrgan($repOrgan); $organMember->setMember($ref->getMember()); @@ -212,7 +212,8 @@ public function generateDischarge(ReportDischargeModel $ref): void if ($rp->isInitialized($ref->getInstallation())) { $organMember = $ref->getInstallation()->getOrganMember(); } else { - $organMember = null; + $organMember = $this->emReport->getRepository(ReportOrganMemberModel::class) + ->findOneBy(['installation' => $ref->getInstallation()]); } if (null === $organMember) { diff --git a/module/Report/src/Service/SubDecision.php b/module/Report/src/Service/SubDecision.php new file mode 100644 index 000000000..5792e6728 --- /dev/null +++ b/module/Report/src/Service/SubDecision.php @@ -0,0 +1,80 @@ +boardService->generateInstallation($subDecision); + break; + + case $subDecision instanceof BoardReleaseModel: + $this->boardService->generateRelease($subDecision); + break; + + case $subDecision instanceof BoardDischargeModel: + $this->boardService->generateDischarge($subDecision); + break; + + // Keyholder-related + case $subDecision instanceof KeyGrantingModel: + $this->keyholderService->generateGranting($subDecision); + break; + + case $subDecision instanceof KeyWithdrawalModel: + $this->keyholderService->generateWithdrawal($subDecision); + break; + + // Organ-related + case $subDecision instanceof FoundationModel: + $this->organService->generateFoundation($subDecision); + break; + + case $subDecision instanceof AbrogationModel: + $this->organService->generateAbrogation($subDecision); + break; + + case $subDecision instanceof InstallationModel: + $this->organService->generateInstallation($subDecision); + break; + + case $subDecision instanceof DischargeModel: + $this->organService->generateDischarge($subDecision); + break; + } + + $this->emReport->persist($subDecision); + } +} diff --git a/phpcs.xml.dist b/phpcs.xml.dist index afed0fe1b..8505f9dd0 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -94,6 +94,7 @@ module/Report/src/Service/Factory/MemberFactory.php module/Report/src/Service/Factory/MiscFactory.php module/Report/src/Service/Factory/OrganFactory.php + module/Report/src/Service/Factory/SubDecisionFactory.php module/User/src/Adapter/Factory/ApiPrincipalAdapterFactory.php module/User/src/Controller/Factory/ApiSettingsControllerFactory.php module/User/src/Controller/Factory/SettingsControllerFactory.php diff --git a/psalm/psalm-baseline.xml b/psalm/psalm-baseline.xml index fe6047d82..1d026f7a3 100644 --- a/psalm/psalm-baseline.xml +++ b/psalm/psalm-baseline.xml @@ -33,11 +33,6 @@ $subdecision - - - ReportMemberModel - - $_SERVER['REQUEST_URI']