From 7f3de62b2c042deeae6df2e3da9a77f1fb78328a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Tue, 9 Jul 2024 23:36:12 +0200 Subject: [PATCH] Move the metadata on each workflow transition from the event subscriber to the entity EntityWorkflow::setStep method The main update is in the setStep method of EntityWorkflow, where parameters are added to capture the transition details. These include the exact transition, the user who made the transition and the time of transition. The WorkflowController extracts this information and put it into the transition's context. The MarkingStore transfer it to the EntityWorkflow::setStep method, and all metadata are recorded within the entities themselve. --- .../Controller/WorkflowController.php | 23 +++++++++- .../Entity/Workflow/EntityWorkflow.php | 9 +++- .../Entity/Workflow/EntityWorkflowTest.php | 44 +++++++++++++++---- .../EntityWorkflowMarkingStoreTest.php | 12 ++++- .../Workflow/EntityWorkflowMarkingStore.php | 6 ++- ...ntityWorkflowTransitionEventSubscriber.php | 6 --- 6 files changed, 81 insertions(+), 19 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php index 3a8626f26..6898d87e4 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php @@ -25,6 +25,7 @@ use Chill\MainBundle\Workflow\EntityWorkflowManager; use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; +use Symfony\Component\Clock\ClockInterface; use Symfony\Component\Form\Extension\Core\Type\FormType; use Symfony\Component\Form\Extension\Core\Type\SubmitType; use Symfony\Component\HttpFoundation\Request; @@ -39,7 +40,18 @@ use Symfony\Contracts\Translation\TranslatorInterface; class WorkflowController extends AbstractController { - public function __construct(private readonly EntityWorkflowManager $entityWorkflowManager, private readonly EntityWorkflowRepository $entityWorkflowRepository, private readonly ValidatorInterface $validator, private readonly PaginatorFactory $paginatorFactory, private readonly Registry $registry, private readonly EntityManagerInterface $entityManager, private readonly TranslatorInterface $translator, private readonly ChillSecurity $security, private readonly \Doctrine\Persistence\ManagerRegistry $managerRegistry) {} + public function __construct( + private readonly EntityWorkflowManager $entityWorkflowManager, + private readonly EntityWorkflowRepository $entityWorkflowRepository, + private readonly ValidatorInterface $validator, + private readonly PaginatorFactory $paginatorFactory, + private readonly Registry $registry, + private readonly EntityManagerInterface $entityManager, + private readonly TranslatorInterface $translator, + private readonly ChillSecurity $security, + private readonly \Doctrine\Persistence\ManagerRegistry $managerRegistry, + private readonly ClockInterface $clock, + ) {} #[Route(path: '/{_locale}/main/workflow/create', name: 'chill_main_workflow_create')] public function create(Request $request): Response @@ -310,7 +322,14 @@ class WorkflowController extends AbstractController throw $this->createAccessDeniedException(sprintf("not allowed to apply transition {$transition}: %s", implode(', ', $msgs))); } - $workflow->apply($entityWorkflow, $transition, ['context' => $stepDTO]); + $byUser = $this->security->getUser(); + + $workflow->apply($entityWorkflow, $transition, [ + 'context' => $stepDTO, + 'byUser' => $byUser, + 'transition' => $transition, + 'transitionAt' => $this->clock->now(), + ]); $this->entityManager->flush(); diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php index 9c314115e..b1a93534d 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php @@ -413,8 +413,15 @@ class EntityWorkflow implements TrackCreationInterface, TrackUpdateInterface * * @return $this */ - public function setStep(string $step, WorkflowTransitionContextDTO $transitionContextDTO): self + public function setStep(string $step, WorkflowTransitionContextDTO $transitionContextDTO, string $transition, \DateTimeImmutable $transitionAt, ?User $byUser = null): self { + $previousStep = $this->getCurrentStep(); + + $previousStep + ->setTransitionAfter($transition) + ->setTransitionAt($transitionAt) + ->setTransitionBy($byUser); + $newStep = new EntityWorkflowStep(); $newStep->setCurrentStep($step); diff --git a/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php b/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php index ec5fa6ae2..30003144f 100644 --- a/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php @@ -11,6 +11,7 @@ declare(strict_types=1); namespace Chill\MainBundle\Tests\Entity\Workflow; +use Chill\MainBundle\Entity\User; use Chill\MainBundle\Entity\Workflow\EntityWorkflow; use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO; use PHPUnit\Framework\TestCase; @@ -26,7 +27,7 @@ final class EntityWorkflowTest extends TestCase { $entityWorkflow = new EntityWorkflow(); - $entityWorkflow->setStep('final', new WorkflowTransitionContextDTO($entityWorkflow)); + $entityWorkflow->setStep('final', new WorkflowTransitionContextDTO($entityWorkflow), 'finalize', new \DateTimeImmutable()); $entityWorkflow->getCurrentStep()->setIsFinal(true); $this->assertTrue($entityWorkflow->isFinal()); @@ -38,16 +39,16 @@ final class EntityWorkflowTest extends TestCase $this->assertFalse($entityWorkflow->isFinal()); - $entityWorkflow->setStep('two', new WorkflowTransitionContextDTO($entityWorkflow)); + $entityWorkflow->setStep('two', new WorkflowTransitionContextDTO($entityWorkflow), 'two', new \DateTimeImmutable()); $this->assertFalse($entityWorkflow->isFinal()); - $entityWorkflow->setStep('previous_final', new WorkflowTransitionContextDTO($entityWorkflow)); + $entityWorkflow->setStep('previous_final', new WorkflowTransitionContextDTO($entityWorkflow), 'three', new \DateTimeImmutable()); $this->assertFalse($entityWorkflow->isFinal()); $entityWorkflow->getCurrentStep()->setIsFinal(true); - $entityWorkflow->setStep('final', new WorkflowTransitionContextDTO($entityWorkflow)); + $entityWorkflow->setStep('final', new WorkflowTransitionContextDTO($entityWorkflow), 'four', new \DateTimeImmutable()); $this->assertTrue($entityWorkflow->isFinal()); } @@ -58,23 +59,50 @@ final class EntityWorkflowTest extends TestCase $this->assertFalse($entityWorkflow->isFreeze()); - $entityWorkflow->setStep('step_one', new WorkflowTransitionContextDTO($entityWorkflow)); + $entityWorkflow->setStep('step_one', new WorkflowTransitionContextDTO($entityWorkflow), 'to_step_one', new \DateTimeImmutable()); $this->assertFalse($entityWorkflow->isFreeze()); - $entityWorkflow->setStep('step_three', new WorkflowTransitionContextDTO($entityWorkflow)); + $entityWorkflow->setStep('step_three', new WorkflowTransitionContextDTO($entityWorkflow), 'to_step_three', new \DateTimeImmutable()); $this->assertFalse($entityWorkflow->isFreeze()); - $entityWorkflow->setStep('freezed', new WorkflowTransitionContextDTO($entityWorkflow)); + $entityWorkflow->setStep('freezed', new WorkflowTransitionContextDTO($entityWorkflow), 'to_freezed', new \DateTimeImmutable()); $entityWorkflow->getCurrentStep()->setFreezeAfter(true); $this->assertTrue($entityWorkflow->isFreeze()); - $entityWorkflow->setStep('after_freeze', new WorkflowTransitionContextDTO($entityWorkflow)); + $entityWorkflow->setStep('after_freeze', new WorkflowTransitionContextDTO($entityWorkflow), 'to_after_freeze', new \DateTimeImmutable()); $this->assertTrue($entityWorkflow->isFreeze()); $this->assertTrue($entityWorkflow->getCurrentStep()->isFreezeAfter()); } + + public function testPreviousStepMetadataAreFilled() + { + $entityWorkflow = new EntityWorkflow(); + $initialStep = $entityWorkflow->getCurrentStep(); + + $entityWorkflow->setStep('step_one', new WorkflowTransitionContextDTO($entityWorkflow), 'to_step_one', new \DateTimeImmutable('2024-01-01'), $user1 = new User()); + + $previous = $entityWorkflow->getCurrentStep(); + + $entityWorkflow->setStep('step_one', new WorkflowTransitionContextDTO($entityWorkflow), 'to_step_two', new \DateTimeImmutable('2024-01-02'), $user2 = new User()); + + $final = $entityWorkflow->getCurrentStep(); + + $stepsChained = $entityWorkflow->getStepsChained(); + + self::assertCount(3, $stepsChained); + self::assertSame($initialStep, $stepsChained[0]); + self::assertSame($previous, $stepsChained[1]); + self::assertSame($final, $stepsChained[2]); + self::assertEquals($user1, $initialStep->getTransitionBy()); + self::assertEquals('2024-01-01', $initialStep->getTransitionAt()?->format('Y-m-d')); + self::assertEquals('to_step_one', $initialStep->getTransitionAfter()); + self::assertEquals($user2, $previous->getTransitionBy()); + self::assertEquals('2024-01-02', $previous->getTransitionAt()?->format('Y-m-d')); + self::assertEquals('to_step_two', $previous->getTransitionAfter()); + } } diff --git a/src/Bundle/ChillMainBundle/Tests/Workflow/EntityWorkflowMarkingStoreTest.php b/src/Bundle/ChillMainBundle/Tests/Workflow/EntityWorkflowMarkingStoreTest.php index a32cefb09..4922cab17 100644 --- a/src/Bundle/ChillMainBundle/Tests/Workflow/EntityWorkflowMarkingStoreTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Workflow/EntityWorkflowMarkingStoreTest.php @@ -39,19 +39,29 @@ class EntityWorkflowMarkingStoreTest extends TestCase { $markingStore = $this->buildMarkingStore(); $workflow = new EntityWorkflow(); + $previousStep = $workflow->getCurrentStep(); $dto = new WorkflowTransitionContextDTO($workflow); $dto->futureCcUsers[] = $user1 = new User(); $dto->futureDestUsers[] = $user2 = new User(); $dto->futureDestEmails[] = $email = 'test@example.com'; - $markingStore->setMarking($workflow, new Marking(['foo' => 1]), ['context' => $dto]); + $markingStore->setMarking($workflow, new Marking(['foo' => 1]), [ + 'context' => $dto, + 'transition' => 'bar_transition', + 'byUser' => $user3 = new User(), + 'transitionAt' => $at = new \DateTimeImmutable(), + ]); $currentStep = $workflow->getCurrentStep(); self::assertEquals('foo', $currentStep->getCurrentStep()); self::assertContains($email, $currentStep->getDestEmail()); self::assertContains($user1, $currentStep->getCcUser()); self::assertContains($user2, $currentStep->getDestUser()); + + self::assertSame($user3, $previousStep->getTransitionBy()); + self::assertSame($at, $previousStep->getTransitionAt()); + self::assertEquals('bar_transition', $previousStep->getTransitionAfter()); } private function buildMarkingStore(): EntityWorkflowMarkingStore diff --git a/src/Bundle/ChillMainBundle/Workflow/EntityWorkflowMarkingStore.php b/src/Bundle/ChillMainBundle/Workflow/EntityWorkflowMarkingStore.php index dca929e86..ee07d7a62 100644 --- a/src/Bundle/ChillMainBundle/Workflow/EntityWorkflowMarkingStore.php +++ b/src/Bundle/ChillMainBundle/Workflow/EntityWorkflowMarkingStore.php @@ -40,10 +40,14 @@ final readonly class EntityWorkflowMarkingStore implements MarkingStoreInterface $next = array_keys($places)[0]; $transitionDTO = $context['context'] ?? null; + $transition = $context['transition']; + $byUser = $context['byUser'] ?? null; + $at = $context['transitionAt']; + if (!$transitionDTO instanceof WorkflowTransitionContextDTO) { throw new \UnexpectedValueException(sprintf('Expected instance of %s', WorkflowTransitionContextDTO::class)); } - $subject->setStep($next, $transitionDTO); + $subject->setStep($next, $transitionDTO, $transition, $at, $byUser); } } diff --git a/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowTransitionEventSubscriber.php b/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowTransitionEventSubscriber.php index ce33ea6d0..9c74c861c 100644 --- a/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowTransitionEventSubscriber.php +++ b/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowTransitionEventSubscriber.php @@ -108,12 +108,6 @@ final readonly class EntityWorkflowTransitionEventSubscriber implements EventSub /** @var EntityWorkflow $entityWorkflow */ $entityWorkflow = $event->getSubject(); - $step = $entityWorkflow->getCurrentStep(); - - $step - ->setTransitionAfter($event->getTransition()->getName()) - ->setTransitionAt(new \DateTimeImmutable('now')) - ->setTransitionBy($this->security->getUser()); $this->chillLogger->info('[workflow] apply transition on entityWorkflow', [ 'relatedEntityClass' => $entityWorkflow->getRelatedEntityClass(),