Move the logic to check if dest users are required to a dedicated constraint

- Create a dedicated constraint to check if the destUsers are required by the applied transition.
- Apply on WorkflowTransitionContextDTO and, if required, use the built-in constraints
- create tests

src/Bundle/ChillMainBundle/Form/WorkflowStepType.php

@@ -25,9 +25,7 @@ use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
 use Symfony\Component\Form\Extension\Core\Type\EmailType;
 use Symfony\Component\Form\FormBuilderInterface;
 use Symfony\Component\OptionsResolver\OptionsResolver;
-use Symfony\Component\Validator\Constraints\Callback;
 use Symfony\Component\Validator\Constraints\NotNull;
-use Symfony\Component\Validator\Context\ExecutionContextInterface;
 use Symfony\Component\Workflow\Registry;
 use Symfony\Component\Workflow\Transition;
 
@@ -206,38 +204,6 @@ class WorkflowStepType extends AbstractType
             ->setDefault('data_class', WorkflowTransitionContextDTO::class)
             ->setRequired('entity_workflow')
             ->setAllowedTypes('entity_workflow', EntityWorkflow::class)
-            ->setDefault('suggested_users', [])
-            ->setDefault('constraints', [
-                new Callback(
-                    function (WorkflowTransitionContextDTO $step, ExecutionContextInterface $context, $payload) {
-                        $workflow = $this->registry->get($step->entityWorkflow, $step->entityWorkflow->getWorkflowName());
-                        $transition = $step->transition;
-                        $toFinal = true;
-
-                        if (null === $transition) {
-                            $context
-                                ->buildViolation('workflow.You must select a next step, pick another decision if no next steps are available');
-                        } else {
-                            foreach ($transition->getTos() as $to) {
-                                $meta = $workflow->getMetadataStore()->getPlaceMetadata($to);
-
-                                if (
-                                    !\array_key_exists('isFinal', $meta) || false === $meta['isFinal']
-                                ) {
-                                    $toFinal = false;
-                                }
-                            }
-                            $destUsers = $step->futureDestUsers;
-
-                            if (!$toFinal && [] === $destUsers) {
-                                $context
-                                    ->buildViolation('workflow.You must add at least one dest user or email')
-                                    ->atPath('future_dest_users')
-                                    ->addViolation();
-                            }
-                        }
-                    }
-                ),
-            ]);
+            ->setDefault('suggested_users', []);
     }
 }

src/Bundle/ChillMainBundle/Tests/Workflow/Validator/TransitionHasDestUserIfRequiredValidatorTest.php

@@ -0,0 +1,209 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * Chill is a software for social workers
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Chill\MainBundle\Tests\Workflow\Validator;
+
+use Chill\MainBundle\Entity\User;
+use Chill\MainBundle\Entity\Workflow\EntityWorkflow;
+use Chill\MainBundle\Workflow\Validator\TransitionHasDestUserIfRequired;
+use Chill\MainBundle\Workflow\Validator\TransitionHasDestUserIfRequiredValidator;
+use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO;
+use Symfony\Component\Validator\Test\ConstraintValidatorTestCase;
+use Symfony\Component\Workflow\DefinitionBuilder;
+use Symfony\Component\Workflow\Metadata\InMemoryMetadataStore;
+use Symfony\Component\Workflow\Registry;
+use Symfony\Component\Workflow\SupportStrategy\WorkflowSupportStrategyInterface;
+use Symfony\Component\Workflow\Transition;
+use Symfony\Component\Workflow\Workflow;
+use Symfony\Component\Workflow\WorkflowInterface;
+
+/**
+ * @internal
+ *
+ * @coversNothing
+ */
+class TransitionHasDestUserIfRequiredValidatorTest extends ConstraintValidatorTestCase
+{
+    private Transition $transitionToSent;
+    private Transition $transitionRegular;
+    private Transition $transitionSignature;
+    private Transition $transitionFinal;
+
+    protected function setUp(): void
+    {
+        $this->transitionToSent = new Transition('send', 'initial', 'sent');
+        $this->transitionSignature = new Transition('signature', 'initial', 'signature');
+        $this->transitionRegular = new Transition('regular', 'initial', 'regular');
+        $this->transitionFinal = new Transition('final', 'initial', 'final');
+
+        parent::setUp();
+    }
+
+    public function testTransitionToRegularWithDestUsersRaiseNoViolation(): void
+    {
+        $dto = $this->buildDto();
+        $dto->transition = $this->transitionRegular;
+        $dto->futureDestUsers = [new User()];
+
+        $constraint = new TransitionHasDestUserIfRequired();
+
+        $this->validator->validate($dto, $constraint);
+
+        self::assertNoViolation();
+    }
+
+    public function testTransitionToRegularWithNoUsersRaiseViolation(): void
+    {
+        $dto = $this->buildDto();
+        $dto->transition = $this->transitionRegular;
+
+        $constraint = new TransitionHasDestUserIfRequired();
+        $constraint->messageDestUserRequired = 'validation_message';
+
+        $this->validator->validate($dto, $constraint);
+
+        self::buildViolation($constraint->messageDestUserRequired)
+            ->setCode($constraint->codeDestUserRequired)
+            ->atPath('property.path.futureDestUsers')
+            ->assertRaised();
+    }
+
+    public function testTransitionToSignatureWithUserRaiseViolation(): void
+    {
+        $dto = $this->buildDto();
+        $dto->transition = $this->transitionSignature;
+        $dto->futureDestUsers = [new User()];
+
+        $constraint = new TransitionHasDestUserIfRequired();
+
+        $this->validator->validate($dto, $constraint);
+
+        self::buildViolation($constraint->messageDestUserNotAuthorized)
+            ->setCode($constraint->codeDestUserNotAuthorized)
+            ->atPath('property.path.futureDestUsers')
+            ->assertRaised();
+    }
+
+    public function testTransitionToExternalSendWithUserRaiseViolation(): void
+    {
+        $dto = $this->buildDto();
+        $dto->transition = $this->transitionToSent;
+        $dto->futureDestUsers = [new User()];
+
+        $constraint = new TransitionHasDestUserIfRequired();
+
+        $this->validator->validate($dto, $constraint);
+
+        self::buildViolation($constraint->messageDestUserNotAuthorized)
+            ->setCode($constraint->codeDestUserNotAuthorized)
+            ->atPath('property.path.futureDestUsers')
+            ->assertRaised();
+    }
+
+    public function testTransitionToFinalWithUserRaiseViolation(): void
+    {
+        $dto = $this->buildDto();
+        $dto->transition = $this->transitionFinal;
+        $dto->futureDestUsers = [new User()];
+
+        $constraint = new TransitionHasDestUserIfRequired();
+
+        $this->validator->validate($dto, $constraint);
+
+        self::buildViolation($constraint->messageDestUserNotAuthorized)
+            ->setCode($constraint->codeDestUserNotAuthorized)
+            ->atPath('property.path.futureDestUsers')
+            ->assertRaised();
+    }
+
+    public function testTransitionToSignatureWithNoUserNoViolation(): void
+    {
+        $dto = $this->buildDto();
+        $dto->transition = $this->transitionSignature;
+
+        $constraint = new TransitionHasDestUserIfRequired();
+
+        $this->validator->validate($dto, $constraint);
+
+        self::assertNoViolation();
+    }
+
+    public function testTransitionToExternalSendWithNoUserNoViolation(): void
+    {
+        $dto = $this->buildDto();
+        $dto->transition = $this->transitionToSent;
+
+        $constraint = new TransitionHasDestUserIfRequired();
+
+        $this->validator->validate($dto, $constraint);
+
+        self::assertNoViolation();
+    }
+
+    public function testTransitionToFinalWithNoUserNoViolation(): void
+    {
+        $dto = $this->buildDto();
+        $dto->transition = $this->transitionFinal;
+
+        $constraint = new TransitionHasDestUserIfRequired();
+
+        $this->validator->validate($dto, $constraint);
+
+        self::assertNoViolation();
+    }
+
+    private function buildDto(): WorkflowTransitionContextDTO
+    {
+        $entityWorkflow = new EntityWorkflow();
+        $entityWorkflow->setWorkflowName('dummy');
+
+        return new WorkflowTransitionContextDTO($entityWorkflow);
+    }
+
+    private function buildRegistry(): Registry
+    {
+        $builder = new DefinitionBuilder(
+            ['initial', 'sent', 'signature', 'regular', 'final'],
+            [
+                $this->transitionToSent,
+                $this->transitionSignature,
+                $this->transitionRegular,
+                $this->transitionFinal,
+            ]
+        );
+        $builder
+            ->setInitialPlaces('initial')
+            ->setMetadataStore(new InMemoryMetadataStore(
+                placesMetadata: [
+                    'sent' => ['isSentExternal' => true],
+                    'signature' => ['isSignature' => ['person', 'user']],
+                    'final' => ['isFinal' => true],
+                ]
+            ))
+        ;
+
+        $workflow = new Workflow($builder->build(), name: 'dummy');
+        $registry = new Registry();
+        $registry->addWorkflow($workflow, new class () implements WorkflowSupportStrategyInterface {
+            public function supports(WorkflowInterface $workflow, object $subject): bool
+            {
+                return true;
+            }
+        });
+
+        return $registry;
+    }
+
+    protected function createValidator(): TransitionHasDestUserIfRequiredValidator
+    {
+        return new TransitionHasDestUserIfRequiredValidator($this->buildRegistry());
+    }
+}

src/Bundle/ChillMainBundle/Workflow/Validator/TransitionHasDestUserIfRequired.php

@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * Chill is a software for social workers
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Chill\MainBundle\Workflow\Validator;
+
+use Symfony\Component\Validator\Constraint;
+
+/**
+ * Check that the next stop has a dest user if this is required by the transition.
+ */
+#[\Attribute]
+class TransitionHasDestUserIfRequired extends Constraint
+{
+    public $messageDestUserRequired = 'workflow.You must add at least one dest user or email';
+    public $codeDestUserRequired = '637c20a6-822c-11ef-a4dd-07b4c0c0efa8';
+    public $messageDestUserNotAuthorized = 'workflow.dest_user_not_authorized';
+    public $codeDestUserNotAuthorized = '8377be2c-822e-11ef-b53a-57ad65828a8e';
+
+    public function getTargets(): string
+    {
+        return self::CLASS_CONSTRAINT;
+    }
+}

src/Bundle/ChillMainBundle/Workflow/Validator/TransitionHasDestUserIfRequiredValidator.php

@@ -0,0 +1,79 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * Chill is a software for social workers
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Chill\MainBundle\Workflow\Validator;
+
+use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO;
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidator;
+use Symfony\Component\Validator\Exception\UnexpectedTypeException;
+use Symfony\Component\Validator\Exception\UnexpectedValueException;
+use Symfony\Component\Workflow\Registry;
+
+final class TransitionHasDestUserIfRequiredValidator extends ConstraintValidator
+{
+    public function __construct(private readonly Registry $registry) {}
+
+    public function validate($value, Constraint $constraint)
+    {
+        if (!$constraint instanceof TransitionHasDestUserIfRequired) {
+            throw new UnexpectedTypeException($constraint, TransitionHasDestUserIfRequired::class);
+        }
+
+        if (!$value instanceof WorkflowTransitionContextDTO) {
+            throw new UnexpectedValueException($value, WorkflowTransitionContextDTO::class);
+        }
+
+        if (null === $value->transition) {
+            return;
+        }
+
+        $workflow = $this->registry->get($value->entityWorkflow, $value->entityWorkflow->getWorkflowName());
+        $metadataStore = $workflow->getMetadataStore();
+
+        $destUsersRequired = false;
+
+        foreach ($value->transition->getTos() as $to) {
+            $metadata = $metadataStore->getPlaceMetadata($to);
+
+            // if the place are only 'isSentExternal' or 'isSignature' or 'final', then, we skip - a destUser is not required
+            if ($metadata['isSentExternal'] ?? false) {
+                continue;
+            }
+            if ($metadata['isSignature'] ?? false) {
+                continue;
+            }
+            if ($metadata['isFinal'] ?? false) {
+                continue;
+            }
+            // if there isn't any 'isSentExternal' or 'isSignature' or final, then we must have a destUser
+            $destUsersRequired = true;
+        }
+
+        if (!$destUsersRequired) {
+            if (0 < count($value->futureDestUsers)) {
+                $this->context->buildViolation($constraint->messageDestUserNotAuthorized)
+                    ->setCode($constraint->codeDestUserNotAuthorized)
+                    ->atPath('futureDestUsers')
+                    ->addViolation();
+            }
+
+            return;
+        }
+
+        if (0 === count($value->futureDestUsers)) {
+            $this->context->buildViolation($constraint->messageDestUserRequired)
+                ->setCode($constraint->codeDestUserRequired)
+                ->atPath('futureDestUsers')
+                ->addViolation();
+        }
+    }
+}

src/Bundle/ChillMainBundle/Workflow/WorkflowTransitionContextDTO.php

@@ -15,6 +15,7 @@ use Chill\MainBundle\Entity\User;
 use Chill\MainBundle\Entity\UserGroup;
 use Chill\MainBundle\Entity\Workflow\EntityWorkflow;
 use Chill\MainBundle\Workflow\Validator\TransitionHasDestineeIfIsSentExternal;
+use Chill\MainBundle\Workflow\Validator\TransitionHasDestUserIfRequired;
 use Chill\PersonBundle\Entity\Person;
 use Chill\ThirdPartyBundle\Entity\ThirdParty;
 use Chill\ThirdPartyBundle\Validator\ThirdPartyHasEmail;
@@ -26,6 +27,7 @@ use Symfony\Component\Workflow\Transition;
  * Context for a transition on an workflow entity.
  */
 #[TransitionHasDestineeIfIsSentExternal]
+#[TransitionHasDestUserIfRequired]
 class WorkflowTransitionContextDTO
 {
     /**
@@ -81,6 +83,7 @@ class WorkflowTransitionContextDTO
     ])]
     public array $futureDestineeEmails = [];
 
+    #[Assert\NotNull]
     public ?Transition $transition = null;
 
     public string $comment = '';