From a309cc077480c1b2889f630098925c9b90476d73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 1 Jul 2024 20:47:15 +0200 Subject: [PATCH] Refactor workflow classes and forms - the workflow controller add a context to each transition; - the state of the entity workflow is applyied using a dedicated marking store - the method EntityWorkflow::step use the context to associate the new step with the future destination user, cc users and email. This makes the step consistent at every step. - this allow to remove some logic which was processed in eventSubscribers, - as counterpart, each workflow must specify a dedicated marking_store: ```yaml framework: workflows: vendee_internal: # ... marking_store: service: Chill\MainBundle\Workflow\EntityWorkflowMarkingStore ``` --- ...ompanyingCourseDocumentWorkflowHandler.php | 5 - .../Controller/WorkflowController.php | 13 +- .../Entity/Workflow/EntityWorkflow.php | 45 +-- .../ChillMainBundle/Form/WorkflowStepType.php | 266 ++++++++---------- .../views/Workflow/_decision.html.twig | 14 +- .../Entity/Workflow/EntityWorkflowTest.php | 17 +- .../EntityWorkflowMarkingStoreTest.php | 61 ++++ .../EntityWorkflowHandlerInterface.php | 2 - .../Workflow/EntityWorkflowMarkingStore.php | 49 ++++ ...ntityWorkflowTransitionEventSubscriber.php | 35 +-- .../NotificationOnTransition.php | 13 +- .../SendAccessKeyEventSubscriber.php | 10 +- .../Workflow/WorkflowTransitionContextDTO.php | 74 +++++ ...dWorkEvaluationDocumentWorkflowHandler.php | 5 - ...ingPeriodWorkEvaluationWorkflowHandler.php | 5 - .../AccompanyingPeriodWorkWorkflowHandler.php | 5 - 16 files changed, 362 insertions(+), 257 deletions(-) create mode 100644 src/Bundle/ChillMainBundle/Tests/Workflow/EntityWorkflowMarkingStoreTest.php create mode 100644 src/Bundle/ChillMainBundle/Workflow/EntityWorkflowMarkingStore.php create mode 100644 src/Bundle/ChillMainBundle/Workflow/WorkflowTransitionContextDTO.php diff --git a/src/Bundle/ChillDocStoreBundle/Workflow/AccompanyingCourseDocumentWorkflowHandler.php b/src/Bundle/ChillDocStoreBundle/Workflow/AccompanyingCourseDocumentWorkflowHandler.php index 9d1b619f9..55b823dcd 100644 --- a/src/Bundle/ChillDocStoreBundle/Workflow/AccompanyingCourseDocumentWorkflowHandler.php +++ b/src/Bundle/ChillDocStoreBundle/Workflow/AccompanyingCourseDocumentWorkflowHandler.php @@ -121,9 +121,4 @@ class AccompanyingCourseDocumentWorkflowHandler implements EntityWorkflowHandler { return AccompanyingCourseDocument::class === $entityWorkflow->getRelatedEntityClass(); } - - public function supportsFreeze(EntityWorkflow $entityWorkflow, array $options = []): bool - { - return false; - } } diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php index d75e42012..3a8626f26 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php @@ -22,6 +22,7 @@ use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use Chill\MainBundle\Security\Authorization\EntityWorkflowVoter; use Chill\MainBundle\Security\ChillSecurity; use Chill\MainBundle\Workflow\EntityWorkflowManager; +use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\Form\Extension\Core\Type\FormType; @@ -279,7 +280,7 @@ class WorkflowController extends AbstractController if (\count($workflow->getEnabledTransitions($entityWorkflow)) > 0) { // possible transition - + $stepDTO = new WorkflowTransitionContextDTO($entityWorkflow); $usersInvolved = $entityWorkflow->getUsersInvolved(); $currentUserFound = array_search($this->security->getUser(), $usersInvolved, true); @@ -289,9 +290,8 @@ class WorkflowController extends AbstractController $transitionForm = $this->createForm( WorkflowStepType::class, - $entityWorkflow->getCurrentStep(), + $stepDTO, [ - 'transition' => true, 'entity_workflow' => $entityWorkflow, 'suggested_users' => $usersInvolved, ] @@ -310,12 +310,7 @@ class WorkflowController extends AbstractController throw $this->createAccessDeniedException(sprintf("not allowed to apply transition {$transition}: %s", implode(', ', $msgs))); } - // TODO symfony 5: add those "future" on context ($workflow->apply($entityWorkflow, $transition, $context) - $entityWorkflow->futureCcUsers = $transitionForm['future_cc_users']->getData() ?? []; - $entityWorkflow->futureDestUsers = $transitionForm['future_dest_users']->getData() ?? []; - $entityWorkflow->futureDestEmails = $transitionForm['future_dest_emails']->getData() ?? []; - - $workflow->apply($entityWorkflow, $transition); + $workflow->apply($entityWorkflow, $transition, ['context' => $stepDTO]); $this->entityManager->flush(); diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php index 6df054b61..9c314115e 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php @@ -17,9 +17,9 @@ use Chill\MainBundle\Doctrine\Model\TrackUpdateInterface; use Chill\MainBundle\Doctrine\Model\TrackUpdateTrait; use Chill\MainBundle\Entity\User; use Chill\MainBundle\Workflow\Validator\EntityWorkflowCreation; +use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; -use Doctrine\Common\Collections\Order; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Serializer\Annotation as Serializer; use Symfony\Component\Validator\Constraints as Assert; @@ -34,35 +34,6 @@ class EntityWorkflow implements TrackCreationInterface, TrackUpdateInterface use TrackUpdateTrait; - /** - * a list of future cc users for the next steps. - * - * @var array|User[] - */ - public array $futureCcUsers = []; - - /** - * a list of future dest emails for the next steps. - * - * This is in used in order to let controller inform who will be the future emails which will validate - * the next step. This is necessary to perform some computation about the next emails, before they are - * associated to the entity EntityWorkflowStep. - * - * @var array|string[] - */ - public array $futureDestEmails = []; - - /** - * a list of future dest users for the next steps. - * - * This is in used in order to let controller inform who will be the future users which will validate - * the next step. This is necessary to perform some computation about the next users, before they are - * associated to the entity EntityWorkflowStep. - * - * @var array|User[] - */ - public array $futureDestUsers = []; - /** * @var Collection */ @@ -442,11 +413,23 @@ class EntityWorkflow implements TrackCreationInterface, TrackUpdateInterface * * @return $this */ - public function setStep(string $step): self + public function setStep(string $step, WorkflowTransitionContextDTO $transitionContextDTO): self { $newStep = new EntityWorkflowStep(); $newStep->setCurrentStep($step); + foreach ($transitionContextDTO->futureCcUsers as $user) { + $newStep->addCcUser($user); + } + + foreach ($transitionContextDTO->futureDestUsers as $user) { + $newStep->addDestUser($user); + } + + foreach ($transitionContextDTO->futureDestEmails as $email) { + $newStep->addDestEmail($email); + } + // copy the freeze if ($this->isFreeze()) { $newStep->setFreezeAfter(true); diff --git a/src/Bundle/ChillMainBundle/Form/WorkflowStepType.php b/src/Bundle/ChillMainBundle/Form/WorkflowStepType.php index 3a3f1b8d3..284781d54 100644 --- a/src/Bundle/ChillMainBundle/Form/WorkflowStepType.php +++ b/src/Bundle/ChillMainBundle/Form/WorkflowStepType.php @@ -12,14 +12,12 @@ declare(strict_types=1); namespace Chill\MainBundle\Form; use Chill\MainBundle\Entity\Workflow\EntityWorkflow; -use Chill\MainBundle\Entity\Workflow\EntityWorkflowStep; use Chill\MainBundle\Form\Type\ChillCollectionType; use Chill\MainBundle\Form\Type\ChillTextareaType; use Chill\MainBundle\Form\Type\PickUserDynamicType; use Chill\MainBundle\Templating\TranslatableStringHelperInterface; -use Chill\MainBundle\Workflow\EntityWorkflowManager; +use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO; use Symfony\Component\Form\AbstractType; -use Symfony\Component\Form\Extension\Core\Type\CheckboxType; use Symfony\Component\Form\Extension\Core\Type\ChoiceType; use Symfony\Component\Form\Extension\Core\Type\EmailType; use Symfony\Component\Form\FormBuilderInterface; @@ -34,169 +32,151 @@ use Symfony\Component\Workflow\Transition; class WorkflowStepType extends AbstractType { - public function __construct(private readonly EntityWorkflowManager $entityWorkflowManager, private readonly Registry $registry, private readonly TranslatableStringHelperInterface $translatableStringHelper) {} + public function __construct( + private readonly Registry $registry, + private readonly TranslatableStringHelperInterface $translatableStringHelper + ) {} public function buildForm(FormBuilderInterface $builder, array $options) { /** @var EntityWorkflow $entityWorkflow */ $entityWorkflow = $options['entity_workflow']; - $handler = $this->entityWorkflowManager->getHandler($entityWorkflow); $workflow = $this->registry->get($entityWorkflow, $entityWorkflow->getWorkflowName()); $place = $workflow->getMarking($entityWorkflow); $placeMetadata = $workflow->getMetadataStore()->getPlaceMetadata(array_keys($place->getPlaces())[0]); - if (true === $options['transition']) { - if (null === $options['entity_workflow']) { - throw new \LogicException('if transition is true, entity_workflow should be defined'); - } + if (null === $options['entity_workflow']) { + throw new \LogicException('if transition is true, entity_workflow should be defined'); + } - $transitions = $this->registry - ->get($options['entity_workflow'], $entityWorkflow->getWorkflowName()) - ->getEnabledTransitions($entityWorkflow); + $transitions = $this->registry + ->get($options['entity_workflow'], $entityWorkflow->getWorkflowName()) + ->getEnabledTransitions($entityWorkflow); - $choices = array_combine( - array_map( - static fn (Transition $transition) => $transition->getName(), - $transitions - ), + $choices = array_combine( + array_map( + static fn (Transition $transition) => $transition->getName(), $transitions - ); + ), + $transitions + ); - if (\array_key_exists('validationFilterInputLabels', $placeMetadata)) { - $inputLabels = $placeMetadata['validationFilterInputLabels']; + if (\array_key_exists('validationFilterInputLabels', $placeMetadata)) { + $inputLabels = $placeMetadata['validationFilterInputLabels']; - $builder->add('transitionFilter', ChoiceType::class, [ - 'multiple' => false, - 'label' => 'workflow.My decision', - 'choices' => [ - 'forward' => 'forward', - 'backward' => 'backward', - 'neutral' => 'neutral', - ], - 'choice_label' => fn (string $key) => $this->translatableStringHelper->localize($inputLabels[$key]), - 'choice_attr' => static fn (string $key) => [ - $key => $key, - ], - 'mapped' => false, - 'expanded' => true, - 'data' => 'forward', - ]); - } - - $builder - ->add('transition', ChoiceType::class, [ - 'label' => 'workflow.Next step', - 'mapped' => false, - 'multiple' => false, - 'expanded' => true, - 'choices' => $choices, - 'constraints' => [new NotNull()], - 'choice_label' => function (Transition $transition) use ($workflow) { - $meta = $workflow->getMetadataStore()->getTransitionMetadata($transition); - - if (\array_key_exists('label', $meta)) { - return $this->translatableStringHelper->localize($meta['label']); - } - - return $transition->getName(); - }, - 'choice_attr' => static function (Transition $transition) use ($workflow) { - $toFinal = true; - $isForward = 'neutral'; - - $metadata = $workflow->getMetadataStore()->getTransitionMetadata($transition); - - if (\array_key_exists('isForward', $metadata)) { - if ($metadata['isForward']) { - $isForward = 'forward'; - } else { - $isForward = 'backward'; - } - } - - foreach ($transition->getTos() as $to) { - $meta = $workflow->getMetadataStore()->getPlaceMetadata($to); - - if ( - !\array_key_exists('isFinal', $meta) || false === $meta['isFinal'] - ) { - $toFinal = false; - } - } - - return [ - 'data-is-transition' => 'data-is-transition', - 'data-to-final' => $toFinal ? '1' : '0', - 'data-is-forward' => $isForward, - ]; - }, - ]) - ->add('future_dest_users', PickUserDynamicType::class, [ - 'label' => 'workflow.dest for next steps', - 'multiple' => true, - 'mapped' => false, - 'suggested' => $options['suggested_users'], - ]) - ->add('future_cc_users', PickUserDynamicType::class, [ - 'label' => 'workflow.cc for next steps', - 'multiple' => true, - 'mapped' => false, - 'required' => false, - 'suggested' => $options['suggested_users'], - ]) - ->add('future_dest_emails', ChillCollectionType::class, [ - 'label' => 'workflow.dest by email', - 'help' => 'workflow.dest by email help', - 'mapped' => false, - 'allow_add' => true, - 'entry_type' => EmailType::class, - 'button_add_label' => 'workflow.Add an email', - 'button_remove_label' => 'workflow.Remove an email', - 'empty_collection_explain' => 'workflow.Any email', - 'entry_options' => [ - 'constraints' => [ - new NotNull(), new NotBlank(), new Email(), - ], - 'label' => 'Email', - ], - ]); + $builder->add('transitionFilter', ChoiceType::class, [ + 'multiple' => false, + 'label' => 'workflow.My decision', + 'choices' => [ + 'forward' => 'forward', + 'backward' => 'backward', + 'neutral' => 'neutral', + ], + 'choice_label' => fn (string $key) => $this->translatableStringHelper->localize($inputLabels[$key]), + 'choice_attr' => static fn (string $key) => [ + $key => $key, + ], + 'mapped' => false, + 'expanded' => true, + 'data' => 'forward', + ]); } - if ( - $handler->supportsFreeze($entityWorkflow) - && !$entityWorkflow->isFreeze() - ) { - $builder - ->add('freezeAfter', CheckboxType::class, [ - 'required' => false, - 'label' => 'workflow.Freeze', - 'help' => 'workflow.The associated element will be freezed', - ]); - } + $builder + ->add('transition', ChoiceType::class, [ + 'label' => 'workflow.Next step', + 'mapped' => false, + 'multiple' => false, + 'expanded' => true, + 'choices' => $choices, + 'constraints' => [new NotNull()], + 'choice_label' => function (Transition $transition) use ($workflow) { + $meta = $workflow->getMetadataStore()->getTransitionMetadata($transition); + + if (\array_key_exists('label', $meta)) { + return $this->translatableStringHelper->localize($meta['label']); + } + + return $transition->getName(); + }, + 'choice_attr' => static function (Transition $transition) use ($workflow) { + $toFinal = true; + $isForward = 'neutral'; + + $metadata = $workflow->getMetadataStore()->getTransitionMetadata($transition); + + if (\array_key_exists('isForward', $metadata)) { + if ($metadata['isForward']) { + $isForward = 'forward'; + } else { + $isForward = 'backward'; + } + } + + foreach ($transition->getTos() as $to) { + $meta = $workflow->getMetadataStore()->getPlaceMetadata($to); + + if ( + !\array_key_exists('isFinal', $meta) || false === $meta['isFinal'] + ) { + $toFinal = false; + } + } + + return [ + 'data-is-transition' => 'data-is-transition', + 'data-to-final' => $toFinal ? '1' : '0', + 'data-is-forward' => $isForward, + ]; + }, + ]) + ->add('futureDestUsers', PickUserDynamicType::class, [ + 'label' => 'workflow.dest for next steps', + 'multiple' => true, + 'suggested' => $options['suggested_users'], + ]) + ->add('futureCcUsers', PickUserDynamicType::class, [ + 'label' => 'workflow.cc for next steps', + 'multiple' => true, + 'required' => false, + 'suggested' => $options['suggested_users'], + ]) + ->add('futureDestEmails', ChillCollectionType::class, [ + 'label' => 'workflow.dest by email', + 'help' => 'workflow.dest by email help', + 'allow_add' => true, + 'entry_type' => EmailType::class, + 'button_add_label' => 'workflow.Add an email', + 'button_remove_label' => 'workflow.Remove an email', + 'empty_collection_explain' => 'workflow.Any email', + 'entry_options' => [ + 'constraints' => [ + new NotNull(), new NotBlank(), new Email(), + ], + 'label' => 'Email', + ], + ]); $builder ->add('comment', ChillTextareaType::class, [ 'required' => false, 'label' => 'Comment', + 'empty_data' => '', ]); } public function configureOptions(OptionsResolver $resolver) { $resolver - ->setDefined('class') - ->setRequired('transition') - ->setAllowedTypes('transition', 'bool') + ->setDefault('data_class', WorkflowTransitionContextDTO::class) ->setRequired('entity_workflow') ->setAllowedTypes('entity_workflow', EntityWorkflow::class) ->setDefault('suggested_users', []) ->setDefault('constraints', [ new Callback( - function ($step, ExecutionContextInterface $context, $payload) { - /** @var EntityWorkflowStep $step */ - $form = $context->getObject(); - $workflow = $this->registry->get($step->getEntityWorkflow(), $step->getEntityWorkflow()->getWorkflowName()); - $transition = $form['transition']->getData(); + function (WorkflowTransitionContextDTO $step, ExecutionContextInterface $context, $payload) { + $workflow = $this->registry->get($step->entityWorkflow, $step->entityWorkflow->getWorkflowName()); + $transition = $step->transition; $toFinal = true; if (null === $transition) { @@ -212,8 +192,8 @@ class WorkflowStepType extends AbstractType $toFinal = false; } } - $destUsers = $form['future_dest_users']->getData(); - $destEmails = $form['future_dest_emails']->getData(); + $destUsers = $step->futureDestUsers; + $destEmails = $step->futureDestEmails; if (!$toFinal && [] === $destUsers && [] === $destEmails) { $context @@ -224,20 +204,6 @@ class WorkflowStepType extends AbstractType } } ), - new Callback( - function ($step, ExecutionContextInterface $context, $payload) { - $form = $context->getObject(); - - foreach ($form->get('future_dest_users')->getData() as $u) { - if (in_array($u, $form->get('future_cc_users')->getData(), true)) { - $context - ->buildViolation('workflow.The user in cc cannot be a dest user in the same workflow step') - ->atPath('ccUsers') - ->addViolation(); - } - } - } - ), ]); } } diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_decision.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_decision.html.twig index 0056c503d..35f587e38 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_decision.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_decision.html.twig @@ -58,17 +58,15 @@ {{ form_row(transition_form.transition) }} - {% if transition_form.freezeAfter is defined %} - {{ form_row(transition_form.freezeAfter) }} - {% endif %} -
- {{ form_row(transition_form.future_dest_users) }} + {{ form_row(transition_form.futureDestUsers) }} + {{ form_errors(transition_form.futureDestUsers) }} - {{ form_row(transition_form.future_cc_users) }} + {{ form_row(transition_form.futureCcUsers) }} + {{ form_errors(transition_form.futureCcUsers) }} - {{ form_row(transition_form.future_dest_emails) }} - {{ form_errors(transition_form.future_dest_users) }} + {{ form_row(transition_form.futureDestEmails) }} + {{ form_errors(transition_form.futureDestEmails) }}

{{ form_label(transition_form.comment) }}

diff --git a/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php b/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php index f515490b8..ec5fa6ae2 100644 --- a/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php @@ -12,6 +12,7 @@ declare(strict_types=1); namespace Chill\MainBundle\Tests\Entity\Workflow; use Chill\MainBundle\Entity\Workflow\EntityWorkflow; +use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO; use PHPUnit\Framework\TestCase; /** @@ -25,7 +26,7 @@ final class EntityWorkflowTest extends TestCase { $entityWorkflow = new EntityWorkflow(); - $entityWorkflow->setStep('final'); + $entityWorkflow->setStep('final', new WorkflowTransitionContextDTO($entityWorkflow)); $entityWorkflow->getCurrentStep()->setIsFinal(true); $this->assertTrue($entityWorkflow->isFinal()); @@ -37,16 +38,16 @@ final class EntityWorkflowTest extends TestCase $this->assertFalse($entityWorkflow->isFinal()); - $entityWorkflow->setStep('two'); + $entityWorkflow->setStep('two', new WorkflowTransitionContextDTO($entityWorkflow)); $this->assertFalse($entityWorkflow->isFinal()); - $entityWorkflow->setStep('previous_final'); + $entityWorkflow->setStep('previous_final', new WorkflowTransitionContextDTO($entityWorkflow)); $this->assertFalse($entityWorkflow->isFinal()); $entityWorkflow->getCurrentStep()->setIsFinal(true); - $entityWorkflow->setStep('final'); + $entityWorkflow->setStep('final', new WorkflowTransitionContextDTO($entityWorkflow)); $this->assertTrue($entityWorkflow->isFinal()); } @@ -57,20 +58,20 @@ final class EntityWorkflowTest extends TestCase $this->assertFalse($entityWorkflow->isFreeze()); - $entityWorkflow->setStep('step_one'); + $entityWorkflow->setStep('step_one', new WorkflowTransitionContextDTO($entityWorkflow)); $this->assertFalse($entityWorkflow->isFreeze()); - $entityWorkflow->setStep('step_three'); + $entityWorkflow->setStep('step_three', new WorkflowTransitionContextDTO($entityWorkflow)); $this->assertFalse($entityWorkflow->isFreeze()); - $entityWorkflow->setStep('freezed'); + $entityWorkflow->setStep('freezed', new WorkflowTransitionContextDTO($entityWorkflow)); $entityWorkflow->getCurrentStep()->setFreezeAfter(true); $this->assertTrue($entityWorkflow->isFreeze()); - $entityWorkflow->setStep('after_freeze'); + $entityWorkflow->setStep('after_freeze', new WorkflowTransitionContextDTO($entityWorkflow)); $this->assertTrue($entityWorkflow->isFreeze()); diff --git a/src/Bundle/ChillMainBundle/Tests/Workflow/EntityWorkflowMarkingStoreTest.php b/src/Bundle/ChillMainBundle/Tests/Workflow/EntityWorkflowMarkingStoreTest.php new file mode 100644 index 000000000..a32cefb09 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Tests/Workflow/EntityWorkflowMarkingStoreTest.php @@ -0,0 +1,61 @@ +buildMarkingStore(); + $workflow = new EntityWorkflow(); + + $marking = $markingStore->getMarking($workflow); + + self::assertEquals(['initial' => 1], $marking->getPlaces()); + } + + public function testSetMarking(): void + { + $markingStore = $this->buildMarkingStore(); + $workflow = new EntityWorkflow(); + + $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]); + + $currentStep = $workflow->getCurrentStep(); + self::assertEquals('foo', $currentStep->getCurrentStep()); + self::assertContains($email, $currentStep->getDestEmail()); + self::assertContains($user1, $currentStep->getCcUser()); + self::assertContains($user2, $currentStep->getDestUser()); + } + + private function buildMarkingStore(): EntityWorkflowMarkingStore + { + return new EntityWorkflowMarkingStore(); + } +} diff --git a/src/Bundle/ChillMainBundle/Workflow/EntityWorkflowHandlerInterface.php b/src/Bundle/ChillMainBundle/Workflow/EntityWorkflowHandlerInterface.php index 1d185ba0e..e79982e1c 100644 --- a/src/Bundle/ChillMainBundle/Workflow/EntityWorkflowHandlerInterface.php +++ b/src/Bundle/ChillMainBundle/Workflow/EntityWorkflowHandlerInterface.php @@ -49,6 +49,4 @@ interface EntityWorkflowHandlerInterface public function isObjectSupported(object $object): bool; public function supports(EntityWorkflow $entityWorkflow, array $options = []): bool; - - public function supportsFreeze(EntityWorkflow $entityWorkflow, array $options = []): bool; } diff --git a/src/Bundle/ChillMainBundle/Workflow/EntityWorkflowMarkingStore.php b/src/Bundle/ChillMainBundle/Workflow/EntityWorkflowMarkingStore.php new file mode 100644 index 000000000..dca929e86 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Workflow/EntityWorkflowMarkingStore.php @@ -0,0 +1,49 @@ +getCurrentStep(); + + return new Marking([$step->getCurrentStep() => 1]); + } + + public function setMarking(object $subject, Marking $marking, array $context = []): void + { + if (!$subject instanceof EntityWorkflow) { + throw new \UnexpectedValueException('Expected instance of EntityWorkflow'); + } + + $places = $marking->getPlaces(); + if (1 < count($places)) { + throw new \LogicException('Expected maximum one place'); + } + $next = array_keys($places)[0]; + + $transitionDTO = $context['context'] ?? null; + if (!$transitionDTO instanceof WorkflowTransitionContextDTO) { + throw new \UnexpectedValueException(sprintf('Expected instance of %s', WorkflowTransitionContextDTO::class)); + } + + $subject->setStep($next, $transitionDTO); + } +} diff --git a/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowTransitionEventSubscriber.php b/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowTransitionEventSubscriber.php index b1fa80458..ce33ea6d0 100644 --- a/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowTransitionEventSubscriber.php +++ b/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowTransitionEventSubscriber.php @@ -21,31 +21,13 @@ use Symfony\Component\Workflow\Event\Event; use Symfony\Component\Workflow\Event\GuardEvent; use Symfony\Component\Workflow\TransitionBlocker; -class EntityWorkflowTransitionEventSubscriber implements EventSubscriberInterface +final readonly class EntityWorkflowTransitionEventSubscriber implements EventSubscriberInterface { - public function __construct(private readonly LoggerInterface $chillLogger, private readonly Security $security, private readonly UserRender $userRender) {} - - public function addDests(Event $event): void - { - if (!$event->getSubject() instanceof EntityWorkflow) { - return; - } - - /** @var EntityWorkflow $entityWorkflow */ - $entityWorkflow = $event->getSubject(); - - foreach ($entityWorkflow->futureCcUsers as $user) { - $entityWorkflow->getCurrentStep()->addCcUser($user); - } - - foreach ($entityWorkflow->futureDestUsers as $user) { - $entityWorkflow->getCurrentStep()->addDestUser($user); - } - - foreach ($entityWorkflow->futureDestEmails as $email) { - $entityWorkflow->getCurrentStep()->addDestEmail($email); - } - } + public function __construct( + private LoggerInterface $chillLogger, + private Security $security, + private UserRender $userRender + ) {} public static function getSubscribedEvents(): array { @@ -53,7 +35,6 @@ class EntityWorkflowTransitionEventSubscriber implements EventSubscriberInterfac 'workflow.transition' => 'onTransition', 'workflow.completed' => [ ['markAsFinal', 2048], - ['addDests', 2048], ], 'workflow.guard' => [ ['guardEntityWorkflow', 0], @@ -99,6 +80,10 @@ class EntityWorkflowTransitionEventSubscriber implements EventSubscriberInterfac public function markAsFinal(Event $event): void { + // NOTE: it is not possible to move this method to the marking store, because + // there is dependency between the Workflow definition and the MarkingStoreInterface (the workflow + // constructor need a MarkingStoreInterface) + if (!$event->getSubject() instanceof EntityWorkflow) { return; } diff --git a/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/NotificationOnTransition.php b/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/NotificationOnTransition.php index e290a567e..386b2eff6 100644 --- a/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/NotificationOnTransition.php +++ b/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/NotificationOnTransition.php @@ -23,7 +23,13 @@ use Symfony\Component\Workflow\Registry; class NotificationOnTransition implements EventSubscriberInterface { - public function __construct(private readonly EntityManagerInterface $entityManager, private readonly \Twig\Environment $engine, private readonly MetadataExtractor $metadataExtractor, private readonly Security $security, private readonly Registry $registry) {} + public function __construct( + private readonly EntityManagerInterface $entityManager, + private readonly \Twig\Environment $engine, + private readonly MetadataExtractor $metadataExtractor, + private readonly Security $security, + private readonly Registry $registry + ) {} public static function getSubscribedEvents(): array { @@ -85,7 +91,10 @@ class NotificationOnTransition implements EventSubscriberInterface 'dest' => $subscriber, 'place' => $place, 'workflow' => $workflow, - 'is_dest' => \in_array($subscriber->getId(), array_map(static fn (User $u) => $u->getId(), $entityWorkflow->futureDestUsers), true), + 'is_dest' => \in_array($subscriber->getId(), array_map( + static fn (User $u) => $u->getId(), + $entityWorkflow->getCurrentStep()->getDestUser()->toArray() + ), true), ]; $notification = new Notification(); diff --git a/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/SendAccessKeyEventSubscriber.php b/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/SendAccessKeyEventSubscriber.php index 90a6e19db..1a4b573d5 100644 --- a/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/SendAccessKeyEventSubscriber.php +++ b/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/SendAccessKeyEventSubscriber.php @@ -20,7 +20,13 @@ use Symfony\Component\Workflow\Registry; class SendAccessKeyEventSubscriber { - public function __construct(private readonly \Twig\Environment $engine, private readonly MetadataExtractor $metadataExtractor, private readonly Registry $registry, private readonly EntityWorkflowManager $entityWorkflowManager, private readonly MailerInterface $mailer) {} + public function __construct( + private readonly \Twig\Environment $engine, + private readonly MetadataExtractor $metadataExtractor, + private readonly Registry $registry, + private readonly EntityWorkflowManager $entityWorkflowManager, + private readonly MailerInterface $mailer + ) {} public function postPersist(EntityWorkflowStep $step): void { @@ -32,7 +38,7 @@ class SendAccessKeyEventSubscriber ); $handler = $this->entityWorkflowManager->getHandler($entityWorkflow); - foreach ($entityWorkflow->futureDestEmails as $emailAddress) { + foreach ($step->getDestEmail() as $emailAddress) { $context = [ 'entity_workflow' => $entityWorkflow, 'dest' => $emailAddress, diff --git a/src/Bundle/ChillMainBundle/Workflow/WorkflowTransitionContextDTO.php b/src/Bundle/ChillMainBundle/Workflow/WorkflowTransitionContextDTO.php new file mode 100644 index 000000000..2a8253523 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Workflow/WorkflowTransitionContextDTO.php @@ -0,0 +1,74 @@ +futureDestUsers as $u) { + if (in_array($u, $this->futureCcUsers, true)) { + $context + ->buildViolation('workflow.The user in cc cannot be a dest user in the same workflow step') + ->atPath('ccUsers') + ->addViolation(); + } + } + } +} diff --git a/src/Bundle/ChillPersonBundle/Workflow/AccompanyingPeriodWorkEvaluationDocumentWorkflowHandler.php b/src/Bundle/ChillPersonBundle/Workflow/AccompanyingPeriodWorkEvaluationDocumentWorkflowHandler.php index 0fc13224e..2912d1c01 100644 --- a/src/Bundle/ChillPersonBundle/Workflow/AccompanyingPeriodWorkEvaluationDocumentWorkflowHandler.php +++ b/src/Bundle/ChillPersonBundle/Workflow/AccompanyingPeriodWorkEvaluationDocumentWorkflowHandler.php @@ -123,9 +123,4 @@ class AccompanyingPeriodWorkEvaluationDocumentWorkflowHandler implements EntityW { return AccompanyingPeriodWorkEvaluationDocument::class === $entityWorkflow->getRelatedEntityClass(); } - - public function supportsFreeze(EntityWorkflow $entityWorkflow, array $options = []): bool - { - return false; - } } diff --git a/src/Bundle/ChillPersonBundle/Workflow/AccompanyingPeriodWorkEvaluationWorkflowHandler.php b/src/Bundle/ChillPersonBundle/Workflow/AccompanyingPeriodWorkEvaluationWorkflowHandler.php index 63b34d1dc..a041b3c37 100644 --- a/src/Bundle/ChillPersonBundle/Workflow/AccompanyingPeriodWorkEvaluationWorkflowHandler.php +++ b/src/Bundle/ChillPersonBundle/Workflow/AccompanyingPeriodWorkEvaluationWorkflowHandler.php @@ -109,9 +109,4 @@ class AccompanyingPeriodWorkEvaluationWorkflowHandler implements EntityWorkflowH { return AccompanyingPeriodWorkEvaluation::class === $entityWorkflow->getRelatedEntityClass(); } - - public function supportsFreeze(EntityWorkflow $entityWorkflow, array $options = []): bool - { - return false; - } } diff --git a/src/Bundle/ChillPersonBundle/Workflow/AccompanyingPeriodWorkWorkflowHandler.php b/src/Bundle/ChillPersonBundle/Workflow/AccompanyingPeriodWorkWorkflowHandler.php index 5c74e5b17..ce146a887 100644 --- a/src/Bundle/ChillPersonBundle/Workflow/AccompanyingPeriodWorkWorkflowHandler.php +++ b/src/Bundle/ChillPersonBundle/Workflow/AccompanyingPeriodWorkWorkflowHandler.php @@ -116,9 +116,4 @@ class AccompanyingPeriodWorkWorkflowHandler implements EntityWorkflowHandlerInte { return AccompanyingPeriodWork::class === $entityWorkflow->getRelatedEntityClass(); } - - public function supportsFreeze(EntityWorkflow $entityWorkflow, array $options = []): bool - { - return false; - } }