From 86ec6f82dacf4b1c7a68e9505bf9403a67958a00 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julien=20Fastr=C3=A9?= <julien.fastre@champs-libres.coop>
Date: Thu, 26 Sep 2024 16:04:53 +0200
Subject: [PATCH] Do not block transition in EntityWorkflow when the user is
 member of a dest user group

- refactor EntityWorkflowGuardTransition + tests
- allow to find easily user within userGroup by adding a dedicated method to UserGroup::contains
---
 .../ChillMainBundle/Entity/UserGroup.php       |  5 +++++
 .../Entity/Workflow/EntityWorkflowStep.php     | 12 +++++++++++-
 .../EntityWorkflowGuardTransitionTest.php      | 10 ++++++++++
 .../EntityWorkflowGuardTransition.php          | 18 ++++++++++++++++++
 4 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/src/Bundle/ChillMainBundle/Entity/UserGroup.php b/src/Bundle/ChillMainBundle/Entity/UserGroup.php
index 588013b65..dddd01c70 100644
--- a/src/Bundle/ChillMainBundle/Entity/UserGroup.php
+++ b/src/Bundle/ChillMainBundle/Entity/UserGroup.php
@@ -141,4 +141,9 @@ class UserGroup
     {
         return true;
     }
+
+    public function contains(User $user): bool
+    {
+        return $this->users->contains($user);
+    }
 }
diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php
index 419b42a83..b64d61b71 100644
--- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php
+++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php
@@ -203,7 +203,9 @@ class EntityWorkflowStep
 
     /**
      * get all the users which are allowed to apply a transition: those added manually, and
-     * those added automatically bu using an access key.
+     * those added automatically by using an access key.
+     *
+     * This method exclude the users associated with user groups
      *
      * @psalm-suppress DuplicateArrayKey
      */
@@ -217,6 +219,14 @@ class EntityWorkflowStep
         );
     }
 
+    /**
+     * @return Collection<int, UserGroup>
+     */
+    public function getDestUserGroups(): Collection
+    {
+        return $this->destUserGroups;
+    }
+
     public function getCcUser(): Collection
     {
         return $this->ccUser;
diff --git a/src/Bundle/ChillMainBundle/Tests/Workflow/EventSubscriber/EntityWorkflowGuardTransitionTest.php b/src/Bundle/ChillMainBundle/Tests/Workflow/EventSubscriber/EntityWorkflowGuardTransitionTest.php
index dd3b12e86..eaef9b0c2 100644
--- a/src/Bundle/ChillMainBundle/Tests/Workflow/EventSubscriber/EntityWorkflowGuardTransitionTest.php
+++ b/src/Bundle/ChillMainBundle/Tests/Workflow/EventSubscriber/EntityWorkflowGuardTransitionTest.php
@@ -12,6 +12,7 @@ declare(strict_types=1);
 namespace Chill\MainBundle\Tests\Workflow\EventSubscriber;
 
 use Chill\MainBundle\Entity\User;
+use Chill\MainBundle\Entity\UserGroup;
 use Chill\MainBundle\Entity\Workflow\EntityWorkflow;
 use Chill\MainBundle\Security\Authorization\EntityWorkflowTransitionVoter;
 use Chill\MainBundle\Templating\Entity\UserRender;
@@ -145,6 +146,11 @@ class EntityWorkflowGuardTransitionTest extends TestCase
         yield [self::buildEntityWorkflow([new User()]), 'transition1', null, false, 'd9e39a18-704c-11ef-b235-8fe0619caee7'];
         yield [self::buildEntityWorkflow([$user = new User()]), 'transition3', $user, false, '5b6b95e0-704d-11ef-a5a9-4b6fc11a8eeb'];
         yield [self::buildEntityWorkflow([$user = new User()]), 'transition3', $user, true, '5b6b95e0-704d-11ef-a5a9-4b6fc11a8eeb'];
+
+        $userGroup = new UserGroup();
+        $userGroup->addUser(new User());
+
+        yield [self::buildEntityWorkflow([$userGroup]), 'transition1', new User(), false, 'f3eeb57c-7532-11ec-9495-e7942a2ac7bc'];
     }
 
     public static function provideValidTransition(): iterable
@@ -159,6 +165,10 @@ class EntityWorkflowGuardTransitionTest extends TestCase
         // transition allowed thanks to permission "apply all transitions"
         yield [self::buildEntityWorkflow([new User()]), 'transition1', new User(), true, 'step1'];
         yield [self::buildEntityWorkflow([new User()]), 'transition2', new User(), true, 'step2'];
+
+        $userGroup = new UserGroup();
+        $userGroup->addUser($u = new User());
+        yield [self::buildEntityWorkflow([$userGroup]), 'transition1', $u, false, 'step1'];
     }
 
     public static function buildEntityWorkflow(array $futureDestUsers): EntityWorkflow
diff --git a/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowGuardTransition.php b/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowGuardTransition.php
index 1c9d26fc7..5f9e0bca5 100644
--- a/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowGuardTransition.php
+++ b/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowGuardTransition.php
@@ -87,6 +87,17 @@ class EntityWorkflowGuardTransition implements EventSubscriberInterface
             return;
         }
 
+        if (!$user instanceof User) {
+            $event->addTransitionBlocker(
+                new TransitionBlocker(
+                    'workflow.Only regular user can apply a transition',
+                    '04fb4f76-7c0e-11ef-afc3-877bad7b0fe7'
+                )
+            );
+
+            return;
+        }
+
         // for users
         if (!in_array('only-dest', $systemTransitions, true)) {
             $event->addTransitionBlocker(
@@ -108,6 +119,13 @@ class EntityWorkflowGuardTransition implements EventSubscriberInterface
                 return;
             }
 
+            // we give a second chance, searching for the presence of the user within userGroups
+            foreach ($entityWorkflow->getCurrentStep()->getDestUserGroups() as $userGroup) {
+                if ($userGroup->contains($user)) {
+                    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',