Refactor workflow guard logic and add internal methods

Removed guard logic from EntityWorkflowTransitionEventSubscriber and created a new EntityWorkflowGuardTransition class for separation of concerns. Marked several setter methods in EntityWorkflowStepSignature as internal to guide proper usage. Added comprehensive tests to ensure the new guard logic functions correctly.

src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php

@@ -105,6 +105,11 @@ class EntityWorkflowStepSignature implements TrackCreationInterface, TrackUpdate
         return $this->state;
     }
 
+    /**
+     * @return $this
+     *
+     * @internal You should not use this method directly, use @see{Chill\MainBundle\Workflow\SignatureStepStateChanger} instead
+     */
     public function setState(EntityWorkflowSignatureStateEnum $state): EntityWorkflowStepSignature
     {
         $this->state = $state;
@@ -117,6 +122,11 @@ class EntityWorkflowStepSignature implements TrackCreationInterface, TrackUpdate
         return $this->stateDate;
     }
 
+    /**
+     * @return $this
+     *
+     * @internal You should not use this method directly, use @see{Chill\MainBundle\Workflow\SignatureStepStateChanger} instead
+     */
     public function setStateDate(?\DateTimeImmutable $stateDate): EntityWorkflowStepSignature
     {
         $this->stateDate = $stateDate;
@@ -129,6 +139,11 @@ class EntityWorkflowStepSignature implements TrackCreationInterface, TrackUpdate
         return $this->zoneSignatureIndex;
     }
 
+    /**
+     * @return $this
+     *
+     * @internal You should not use this method directly, use @see{Chill\MainBundle\Workflow\SignatureStepStateChanger} instead
+     */
     public function setZoneSignatureIndex(?int $zoneSignatureIndex): EntityWorkflowStepSignature
     {
         $this->zoneSignatureIndex = $zoneSignatureIndex;

src/Bundle/ChillMainBundle/Tests/Workflow/EventSubscriber/EntityWorkflowGuardTransitionTest.php

@@ -0,0 +1,168 @@
+<?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\EventSubscriber;
+
+use Chill\MainBundle\Entity\User;
+use Chill\MainBundle\Entity\Workflow\EntityWorkflow;
+use Chill\MainBundle\Templating\Entity\UserRender;
+use Chill\MainBundle\Workflow\EntityWorkflowMarkingStore;
+use Chill\MainBundle\Workflow\EventSubscriber\EntityWorkflowGuardTransition;
+use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO;
+use PHPUnit\Framework\TestCase;
+use Prophecy\Argument;
+use Prophecy\PhpUnit\ProphecyTrait;
+use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\Security\Core\Security;
+use Symfony\Component\Workflow\DefinitionBuilder;
+use Symfony\Component\Workflow\Exception\NotEnabledTransitionException;
+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 EntityWorkflowGuardTransitionTest extends TestCase
+{
+    use ProphecyTrait;
+
+    public static function buildRegistry(?EventSubscriberInterface $eventSubscriber): Registry
+    {
+        $builder = new DefinitionBuilder();
+        $builder
+            ->setInitialPlaces(['initial'])
+            ->addPlaces(['initial', 'intermediate', 'step1', 'step2', 'step3'])
+            ->addTransition(new Transition('intermediate', 'initial', 'intermediate'))
+            ->addTransition($transition1 = new Transition('transition1', 'intermediate', 'step1'))
+            ->addTransition($transition2 = new Transition('transition2', 'intermediate', 'step2'))
+            ->addTransition($transition3 = new Transition('transition3', 'intermediate', 'step3'))
+        ;
+
+        $transitionMetadata = new \SplObjectStorage();
+        $transitionMetadata->attach($transition1, ['transitionGuard' => 'only-dest']);
+        $transitionMetadata->attach($transition2, ['transitionGuard' => 'only-dest+system']);
+        $transitionMetadata->attach($transition3, ['transitionGuard' => 'system']);
+
+        $builder->setMetadataStore(new InMemoryMetadataStore(transitionsMetadata: $transitionMetadata));
+
+        if (null !== $eventSubscriber) {
+            $eventDispatcher = new EventDispatcher();
+            $eventDispatcher->addSubscriber($eventSubscriber);
+        }
+
+        $workflow = new Workflow($builder->build(), new EntityWorkflowMarkingStore(), $eventDispatcher ?? null, 'dummy');
+
+        $registry = new Registry();
+        $registry->addWorkflow(
+            $workflow,
+            new class () implements WorkflowSupportStrategyInterface {
+                public function supports(WorkflowInterface $workflow, object $subject): bool
+                {
+                    return true;
+                }
+            }
+        );
+
+        return $registry;
+    }
+
+    /**
+     * @dataProvider provideBlockingTransition
+     */
+    public function testTransitionGuardBlocked(EntityWorkflow $entityWorkflow, string $transition, ?User $user, string $uuid): void
+    {
+        $userRender = $this->prophesize(UserRender::class);
+        $userRender->renderString(Argument::type(User::class), [])->willReturn('user-as-string');
+        $security = $this->prophesize(Security::class);
+        $security->getUser()->willReturn($user);
+
+        $transitionGuard = new EntityWorkflowGuardTransition($userRender->reveal(), $security->reveal());
+        $registry = self::buildRegistry($transitionGuard);
+
+        $workflow = $registry->get($entityWorkflow, 'dummy');
+
+        $context = new WorkflowTransitionContextDTO($entityWorkflow);
+
+        self::expectException(NotEnabledTransitionException::class);
+        try {
+            $workflow->apply($entityWorkflow, $transition, ['context' => $context, 'byUser' => $user, 'transition' => $transition, 'transitionAt' => new \DateTimeImmutable('now')]);
+        } catch (NotEnabledTransitionException $e) {
+            $list = $e->getTransitionBlockerList();
+
+            self::assertEquals(1, $list->count());
+            $list = iterator_to_array($list->getIterator());
+            self::assertEquals($uuid, $list[0]->getCode());
+
+            throw $e;
+        }
+    }
+
+    /**
+     * @dataProvider provideValidTransition
+     */
+    public function testTransitionGuardValid(EntityWorkflow $entityWorkflow, string $transition, ?User $user, string $newStep): void
+    {
+        $userRender = $this->prophesize(UserRender::class);
+        $userRender->renderString(Argument::type(User::class), [])->willReturn('user-as-string');
+        $security = $this->prophesize(Security::class);
+        $security->getUser()->willReturn($user);
+
+        $transitionGuard = new EntityWorkflowGuardTransition($userRender->reveal(), $security->reveal());
+        $registry = self::buildRegistry($transitionGuard);
+
+        $workflow = $registry->get($entityWorkflow, 'dummy');
+        $context = new WorkflowTransitionContextDTO($entityWorkflow);
+
+        $workflow->apply($entityWorkflow, $transition, ['context' => $context, 'byUser' => $user, 'transition' => $transition, 'transitionAt' => new \DateTimeImmutable('now')]);
+
+        self::assertEquals($newStep, $entityWorkflow->getStep());
+    }
+
+    public static function provideBlockingTransition(): iterable
+    {
+        yield [self::buildEntityWorkflow([new User()]), 'transition1', new User(), 'f3eeb57c-7532-11ec-9495-e7942a2ac7bc'];
+        yield [self::buildEntityWorkflow([]), 'transition1', null, 'd9e39a18-704c-11ef-b235-8fe0619caee7'];
+        yield [self::buildEntityWorkflow([new User()]), 'transition1', null, 'd9e39a18-704c-11ef-b235-8fe0619caee7'];
+        yield [self::buildEntityWorkflow([$user = new User()]), 'transition3', $user, '5b6b95e0-704d-11ef-a5a9-4b6fc11a8eeb'];
+    }
+
+    public static function provideValidTransition(): iterable
+    {
+        yield [self::buildEntityWorkflow([$u = new User()]), 'transition1', $u, 'step1'];
+        yield [self::buildEntityWorkflow([$u = new User()]), 'transition2', $u, 'step2'];
+        yield [self::buildEntityWorkflow([new User()]), 'transition2', null, 'step2'];
+        yield [self::buildEntityWorkflow([]), 'transition2', null, 'step2'];
+        yield [self::buildEntityWorkflow([new User()]), 'transition3', null, 'step3'];
+        yield [self::buildEntityWorkflow([]), 'transition3', null, 'step3'];
+    }
+
+    public static function buildEntityWorkflow(array $futureDestUsers): EntityWorkflow
+    {
+        $registry = self::buildRegistry(null);
+        $baseContext = ['transition' => 'intermediate', 'transitionAt' => new \DateTimeImmutable()];
+
+        // test a user not is destination is blocked
+        $entityWorkflow = new EntityWorkflow();
+        $workflow = $registry->get($entityWorkflow, 'dummy');
+        $dto = new WorkflowTransitionContextDTO($entityWorkflow);
+        $dto->futureDestUsers = $futureDestUsers;
+        $workflow->apply($entityWorkflow, 'intermediate', ['context' => $dto, ...$baseContext]);
+
+        return $entityWorkflow;
+    }
+}

src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowGuardTransition.php

@@ -0,0 +1,105 @@
+<?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\EventSubscriber;
+
+use Chill\MainBundle\Entity\User;
+use Chill\MainBundle\Entity\Workflow\EntityWorkflow;
+use Chill\MainBundle\Templating\Entity\UserRender;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\Security\Core\Security;
+use Symfony\Component\Workflow\Event\GuardEvent;
+use Symfony\Component\Workflow\TransitionBlocker;
+
+class EntityWorkflowGuardTransition implements EventSubscriberInterface
+{
+    public function __construct(
+        private readonly UserRender $userRender,
+        private readonly Security $security,
+    ) {}
+
+    public static function getSubscribedEvents(): array
+    {
+        return [
+            'workflow.guard' => [
+                ['guardEntityWorkflow', 0],
+            ],
+        ];
+    }
+
+    public function guardEntityWorkflow(GuardEvent $event)
+    {
+        if (!$event->getSubject() instanceof EntityWorkflow) {
+            return;
+        }
+
+        /** @var EntityWorkflow $entityWorkflow */
+        $entityWorkflow = $event->getSubject();
+
+        if ($entityWorkflow->isFinal()) {
+            $event->addTransitionBlocker(
+                new TransitionBlocker(
+                    'workflow.The workflow is finalized',
+                    'd6306280-7535-11ec-a40d-1f7bee26e2c0'
+                )
+            );
+
+            return;
+        }
+
+        $user = $this->security->getUser();
+        $metadata = $event->getWorkflow()->getMetadataStore()->getTransitionMetadata($event->getTransition());
+        $systemTransitions = explode('+', $metadata['transitionGuard'] ?? 'only-dest');
+
+        if (null === $user) {
+            if (in_array('system', $systemTransitions, true)) {
+                // it is safe to apply this transition
+                return;
+            }
+
+            $event->addTransitionBlocker(
+                new TransitionBlocker(
+                    'workflow.Transition is not allowed for system',
+                    'd9e39a18-704c-11ef-b235-8fe0619caee7'
+                )
+            );
+
+            return;
+        }
+
+        // for users
+        if (!in_array('only-dest', $systemTransitions, true)) {
+            $event->addTransitionBlocker(
+                new TransitionBlocker(
+                    'workflow.Only system can apply this transition',
+                    '5b6b95e0-704d-11ef-a5a9-4b6fc11a8eeb'
+                )
+            );
+        }
+
+        if (!$entityWorkflow->getCurrentStep()->getAllDestUser()->contains($user)) {
+            if ($event->getMarking()->has('initial')) {
+                return;
+            }
+
+            $event->addTransitionBlocker(new TransitionBlocker(
+                'workflow.You are not allowed to apply a transition on this workflow. Only those users are allowed: %users%',
+                'f3eeb57c-7532-11ec-9495-e7942a2ac7bc',
+                [
+                    '%users%' => implode(
+                        ', ',
+                        $entityWorkflow->getCurrentStep()->getAllDestUser()->map(fn (User $u) => $this->userRender->renderString($u, []))->toArray()
+                    ),
+                ]
+            ));
+        }
+    }
+}

src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowTransitionEventSubscriber.php

@@ -13,20 +13,16 @@ namespace Chill\MainBundle\Workflow\EventSubscriber;
 
 use Chill\MainBundle\Entity\User;
 use Chill\MainBundle\Entity\Workflow\EntityWorkflow;
-use Chill\MainBundle\Templating\Entity\UserRender;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\Security\Core\Security;
 use Symfony\Component\Workflow\Event\Event;
-use Symfony\Component\Workflow\Event\GuardEvent;
-use Symfony\Component\Workflow\TransitionBlocker;
 
 final readonly class EntityWorkflowTransitionEventSubscriber implements EventSubscriberInterface
 {
     public function __construct(
         private LoggerInterface $chillLogger,
         private Security $security,
-        private UserRender $userRender,
     ) {}
 
     public static function getSubscribedEvents(): array
@@ -36,48 +32,9 @@ final readonly class EntityWorkflowTransitionEventSubscriber implements EventSub
             'workflow.completed' => [
                 ['markAsFinal', 2048],
             ],
-            'workflow.guard' => [
-                ['guardEntityWorkflow', 0],
-            ],
         ];
     }
 
-    public function guardEntityWorkflow(GuardEvent $event)
-    {
-        if (!$event->getSubject() instanceof EntityWorkflow) {
-            return;
-        }
-
-        /** @var EntityWorkflow $entityWorkflow */
-        $entityWorkflow = $event->getSubject();
-
-        if ($entityWorkflow->isFinal()) {
-            $event->addTransitionBlocker(
-                new TransitionBlocker(
-                    'workflow.The workflow is finalized',
-                    'd6306280-7535-11ec-a40d-1f7bee26e2c0'
-                )
-            );
-
-            return;
-        }
-
-        if (!$entityWorkflow->getCurrentStep()->getAllDestUser()->contains($this->security->getUser())) {
-            if (!$event->getMarking()->has('initial')) {
-                $event->addTransitionBlocker(new TransitionBlocker(
-                    'workflow.You are not allowed to apply a transition on this workflow. Only those users are allowed: %users%',
-                    'f3eeb57c-7532-11ec-9495-e7942a2ac7bc',
-                    [
-                        '%users%' => implode(
-                            ', ',
-                            $entityWorkflow->getCurrentStep()->getAllDestUser()->map(fn (User $u) => $this->userRender->renderString($u, []))->toArray()
-                        ),
-                    ]
-                ));
-            }
-        }
-    }
-
     public function markAsFinal(Event $event): void
     {
         // NOTE: it is not possible to move this method to the marking store, because
@@ -109,11 +66,13 @@ final readonly class EntityWorkflowTransitionEventSubscriber implements EventSub
         /** @var EntityWorkflow $entityWorkflow */
         $entityWorkflow = $event->getSubject();
 
+        $user = $this->security->getUser();
+
         $this->chillLogger->info('[workflow] apply transition on entityWorkflow', [
             'relatedEntityClass' => $entityWorkflow->getRelatedEntityClass(),
             'relatedEntityId' => $entityWorkflow->getRelatedEntityId(),
             'transition' => $event->getTransition()->getName(),
-            'by_user' => $this->security->getUser(),
+            'by_user' => $user instanceof User ? $user->getId() : (string) $user?->getUserIdentifier(),
             'entityWorkflow' => $entityWorkflow->getId(),
         ]);
     }