Fix: Workflow: the notification is send when the user is added on first

step.

See https://gitlab.com/champs-libres/departement-de-la-vendee/accent-suivi-developpement/-/issues/813

CHANGELOG.md

@@ -11,6 +11,12 @@ and this project adheres to
 ## Unreleased
 
 <!-- write down unreleased development here -->
+* [workflow]: Fixed: the notification is sent when the user is added to the first step.
+
+## Test releases
+
+### 2022-06
+
 * [workflow]: added pagination to workflow list page
 * [homepage_widget]: null error on tasks widget fixed
 * [person-thirdparty]: fix quick-add of names that consist of multiple parts (eg. De Vlieger) within onthefly modal person/thirdparty
@@ -19,8 +25,6 @@ and this project adheres to
 * [household]: Reposition and cut button for enfant hors menage have been deleted (https://gitlab.com/champs-libres/departement-de-la-vendee/chill/-/issues/620)
 * [admin]: Add crud for composition type in admin (https://gitlab.com/champs-libres/departement-de-la-vendee/chill/-/issues/611)
 
-## Test releases
-
 ### 2022-05-30
 
 * fix creating a new AccompanyingPeriodWorkEvaluationDocument when replacing the document (the workflow was lost)

src/Bundle/ChillMainBundle/Controller/WorkflowController.php

@@ -318,19 +318,12 @@ class WorkflowController extends AbstractController
                     );
                 }
 
+                // TODO symfony 5: add those "future" on context ($workflow->apply($entityWorkflow, $transition, $context)
                 $entityWorkflow->futureDestUsers = $transitionForm['future_dest_users']->getData();
                 $entityWorkflow->futureDestEmails = $transitionForm['future_dest_emails']->getData();
 
                 $workflow->apply($entityWorkflow, $transition);
 
-                foreach ($transitionForm['future_dest_users']->getData() as $user) {
-                    $entityWorkflow->getCurrentStep()->addDestUser($user);
-                }
-
-                foreach ($transitionForm['future_dest_emails']->getData() as $email) {
-                    $entityWorkflow->getCurrentStep()->addDestEmail($email);
-                }
-
                 $this->entityManager->flush();
 
                 return $this->redirectToRoute('chill_main_workflow_show', ['id' => $entityWorkflow->getId()]);

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

@@ -0,0 +1,102 @@
+<?php
+
+namespace Chill\MainBundle\Tests\Workflow\EventSubscriber;
+
+use Chill\MainBundle\Entity\Notification;
+use Chill\MainBundle\Entity\User;
+use Chill\MainBundle\Entity\Workflow\EntityWorkflow;
+use Chill\MainBundle\Entity\Workflow\EntityWorkflowStep;
+use Chill\MainBundle\Workflow\EventSubscriber\NotificationOnTransition;
+use Chill\MainBundle\Workflow\Helper\MetadataExtractor;
+use Doctrine\ORM\EntityManagerInterface;
+use PHPUnit\Framework\TestCase;
+use Prophecy\Argument;
+use Prophecy\Call\Call;
+use Prophecy\Exception\Prediction\FailedPredictionException;
+use Prophecy\PhpUnit\ProphecyTrait;
+use Symfony\Component\Security\Core\Security;
+use Symfony\Component\Templating\EngineInterface;
+use Symfony\Component\Workflow\Event\Event;
+use Symfony\Component\Workflow\Marking;
+use Symfony\Component\Workflow\Registry;
+use Symfony\Component\Workflow\Transition;
+use Symfony\Component\Workflow\Workflow;
+use Symfony\Component\Workflow\WorkflowInterface;
+
+class NotificationOnTransitionTest extends TestCase
+{
+    use ProphecyTrait;
+
+    public function testOnCompleteSendNotification(): void
+    {
+        $dest = new User();
+        $currentUser = new User();
+        $workflowProphecy = $this->prophesize(WorkflowInterface::class);
+        $workflow = $workflowProphecy->reveal();
+        $entityWorkflow = new EntityWorkflow();
+        $entityWorkflow
+            ->setWorkflowName('workflow_name')
+            ->setRelatedEntityClass(\stdClass::class)
+            ->setRelatedEntityId(1)
+        ;
+        // force an id to entityWorkflow:
+        $reflection = new \ReflectionClass($entityWorkflow);
+        $id = $reflection->getProperty('id');
+        $id->setAccessible(true);
+        $id->setValue($entityWorkflow, 1);
+
+        $step = new EntityWorkflowStep();
+        $entityWorkflow->addStep($step);
+        $step->addDestUser($dest)
+            ->setCurrentStep('to_state')
+        ;
+
+        $em = $this->prophesize(EntityManagerInterface::class);
+        $em->persist(Argument::type(Notification::class))->should(
+            function($args) use ($dest) {
+                /** @var Call[] $args */
+                if (1 !== count($args)) {
+                    throw new FailedPredictionException('no notification sent');
+                }
+
+                $notification = $args[0]->getArguments()[0];
+
+                if (!$notification instanceof Notification) {
+                    throw new FailedPredictionException('persist is not a notification');
+                }
+
+                if (!$notification->getAddressees()->contains($dest)) {
+                    throw new FailedPredictionException('the dest is not notified');
+                }
+            });
+
+        $engine = $this->prophesize(EngineInterface::class);
+        $engine->render(Argument::type('string'), Argument::type('array'))
+            ->willReturn('dummy text');
+
+        $extractor = $this->prophesize(MetadataExtractor::class);
+        $extractor->buildArrayPresentationForPlace(Argument::type(EntityWorkflow::class), Argument::any())
+            ->willReturn([]);
+        $extractor->buildArrayPresentationForWorkflow(Argument::any())
+            ->willReturn([]);
+
+        $registry = $this->prophesize(Registry::class);
+        $registry->get(Argument::type(EntityWorkflow::class), Argument::type('string'))
+            ->willReturn($workflow);
+
+        $security = $this->prophesize(Security::class);
+        $security->getUser()->willReturn($currentUser);
+
+        $notificationOnTransition = new NotificationOnTransition(
+            $em->reveal(),
+            $engine->reveal(),
+            $extractor->reveal(),
+            $security->reveal(),
+            $registry->reveal()
+        );
+
+        $event = new Event($entityWorkflow, new Marking(), new Transition('dummy_transition', ['from_state'], ['to_state']), $workflow);
+
+        $notificationOnTransition->onCompletedSendNotification($event);
+    }
+}

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

@@ -45,13 +45,33 @@ class EntityWorkflowTransitionEventSubscriber implements EventSubscriberInterfac
     {
         return [
             'workflow.transition' => 'onTransition',
-            'workflow.completed' => 'onCompleted',
+            'workflow.completed' => [
+                ['markAsFinal', 2048],
+                ['addDests', 2048],
+            ],
             'workflow.guard' => [
                 ['guardEntityWorkflow', 0],
             ],
         ];
     }
 
+    public function addDests(Event $event): void
+    {
+        if (!$event->getSubject() instanceof EntityWorkflow) {
+            return;
+        }
+
+        /** @var EntityWorkflow $entityWorkflow */
+        $entityWorkflow = $event->getSubject();
+        foreach ($entityWorkflow->futureDestUsers as $user) {
+            $entityWorkflow->getCurrentStep()->addDestUser($user);
+        }
+
+        foreach ($entityWorkflow->futureDestEmails as $email) {
+            $entityWorkflow->getCurrentStep()->addDestEmail($email);
+        }
+    }
+
     public function guardEntityWorkflow(GuardEvent $event)
     {
         if (!$event->getSubject() instanceof EntityWorkflow) {
@@ -90,7 +110,7 @@ class EntityWorkflowTransitionEventSubscriber implements EventSubscriberInterfac
         }
     }
 
-    public function onCompleted(Event $event): void
+    public function markAsFinal(Event $event): void
     {
         if (!$event->getSubject() instanceof EntityWorkflow) {
             return;

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

@@ -52,10 +52,20 @@ class NotificationOnTransition implements EventSubscriberInterface
     public static function getSubscribedEvents(): array
     {
         return [
-            'workflow.completed' => 'onCompletedSendNotification',
+            'workflow.completed' => ['onCompletedSendNotification', 2048],
         ];
     }
 
+    /**
+     * Send a notification to:
+     *
+     * * the dests of the new step;
+     * * the users which subscribed to workflow, on each step, or on final
+     *
+     * **Warning** take care that this method must be executed **after** the dest users are added to
+     * the step (@link{EntityWorkflowStep::addDestUser}). Currently, this is done during
+     * @link{EntityWorkflowTransitionEventSubscriber::addDests}.
+     */
     public function onCompletedSendNotification(Event $event): void
     {
         if (!$event->getSubject() instanceof EntityWorkflow) {
@@ -65,23 +75,27 @@ class NotificationOnTransition implements EventSubscriberInterface
         /** @var EntityWorkflow $entityWorkflow */
         $entityWorkflow = $event->getSubject();
 
-        $dests = array_merge(
+        /** @var array<string, User> $dests array of unique values, where keys is the object's hash */
+        $dests = [];
+        foreach (array_merge(
+            // the subscriber to each step
             $entityWorkflow->getSubscriberToStep()->toArray(),
+            // the subscriber to final, only if final
             $entityWorkflow->isFinal() ? $entityWorkflow->getSubscriberToFinal()->toArray() : [],
-            $entityWorkflow->getCurrentStepChained()->getPrevious()->getDestUser()->toArray()
+            // the dests for the current step
-        );
+            $entityWorkflow->getCurrentStep()->getDestUser()->toArray()
+        ) as $dest) {
+            $dests[spl_object_hash($dest)] = $dest;
+        }
 
         $place = $this->metadataExtractor->buildArrayPresentationForPlace($entityWorkflow);
         $workflow = $this->metadataExtractor->buildArrayPresentationForWorkflow(
             $this->registry->get($entityWorkflow, $entityWorkflow->getWorkflowName())
         );
 
-        $visited = [];
-
         foreach ($dests as $subscriber) {
             if (
                 $this->security->getUser() === $subscriber
-                || in_array($subscriber->getId(), $visited, true)
             ) {
                 continue;
             }
@@ -102,8 +116,6 @@ class NotificationOnTransition implements EventSubscriberInterface
                 ->setMessage($this->engine->render('@ChillMain/Workflow/workflow_notification_on_transition_completed_content.fr.txt.twig', $context))
                 ->addAddressee($subscriber);
             $this->entityManager->persist($notification);
-
-            $visited[] = $subscriber->getId();
         }
     }
 }