Refactor workflow hold functionality to avoid relying on database to perform some checks on "user holds entityworkflow"

Simplify the logic in handling workflow on hold status by moving related checks and operations to `EntityWorkflowStep` and `EntityWorkflow` entities. This includes implementing new methods to check if a step or workflow is held by a specific user and refactoring the controller actions to use these methods.

src/Bundle/ChillMainBundle/Controller/WorkflowController.php

@@ -19,7 +19,6 @@ use Chill\MainBundle\Form\WorkflowStepType;
 use Chill\MainBundle\Pagination\PaginatorFactory;
 use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository;
 use Chill\MainBundle\Repository\Workflow\EntityWorkflowStepSignatureRepository;
-use Chill\MainBundle\Repository\Workflow\EntityWorkflowStepHoldRepository;
 use Chill\MainBundle\Security\Authorization\EntityWorkflowVoter;
 use Chill\MainBundle\Security\ChillSecurity;
 use Chill\MainBundle\Workflow\EntityWorkflowManager;
@@ -54,7 +53,6 @@ class WorkflowController extends AbstractController
         private readonly ChillSecurity $security,
         private readonly \Doctrine\Persistence\ManagerRegistry $managerRegistry,
         private readonly ClockInterface $clock,
-        private readonly EntityWorkflowStepHoldRepository $entityWorkflowStepHoldRepository,
         private readonly EntityWorkflowStepSignatureRepository $entityWorkflowStepSignatureRepository,
     ) {}
 
@@ -298,7 +296,6 @@ class WorkflowController extends AbstractController
         $workflow = $this->registry->get($entityWorkflow, $entityWorkflow->getWorkflowName());
         $errors = [];
         $signatures = $entityWorkflow->getCurrentStep()->getSignatures();
-        $onHoldStep = $this->entityWorkflowStepHoldRepository->findByWorkflow($entityWorkflow);
 
         if (\count($workflow->getEnabledTransitions($entityWorkflow)) > 0) {
             // possible transition
@@ -343,10 +340,6 @@ class WorkflowController extends AbstractController
 
                 $this->entityManager->flush();
 
-                if (null !== $onHoldStep) {
-                    $this->entityManager->remove($onHoldStep);
-                }
-
                 return $this->redirectToRoute('chill_main_workflow_show', ['id' => $entityWorkflow->getId()]);
             }
 
@@ -365,7 +358,6 @@ class WorkflowController extends AbstractController
                 'entity_workflow' => $entityWorkflow,
                 'transition_form_errors' => $errors,
                 'signatures' => $signatures,
-                'onHoldStep' => $onHoldStep,
             ]
         );
     }

src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php

@@ -13,6 +13,7 @@ namespace Chill\MainBundle\Controller;
 
 use Chill\MainBundle\Entity\User;
 use Chill\MainBundle\Entity\Workflow\EntityWorkflow;
+use Chill\MainBundle\Entity\Workflow\EntityWorkflowStep;
 use Chill\MainBundle\Entity\Workflow\EntityWorkflowStepHold;
 use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository;
 use Chill\MainBundle\Repository\Workflow\EntityWorkflowStepHoldRepository;
@@ -21,6 +22,7 @@ use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Symfony\Component\Routing\Annotation\Route;
 use Symfony\Component\Security\Core\Security;
 use Symfony\Component\Workflow\Registry;
@@ -62,20 +64,29 @@ class WorkflowOnHoldController extends AbstractController
         return $this->redirectToRoute('chill_main_workflow_show', ['id' => $entityWorkflow->getId()]);
     }
 
-    #[Route(path: '/{_locale}/main/workflow/{holdId}/remove_hold', name: 'chill_main_workflow_remove_hold')]
+    #[Route(path: '/{_locale}/main/workflow/{id}/remove_hold', name: 'chill_main_workflow_remove_hold')]
-    public function removeOnHold(int $holdId, Request $request): Response
+    public function removeOnHold(EntityWorkflowStep $entityWorkflowStep, Request $request): Response
     {
-        $hold = $this->entityWorkflowStepHoldRepository->findById($holdId);
+        $user = $this->security->getUser();
-        $entityWorkflow = $hold->getStep()->getEntityWorkflow();
-        $currentUser = $this->security->getUser();
 
-        if ($hold->getByUser() !== $currentUser) {
+        if (!$user instanceof User) {
-            throw $this->createAccessDeniedException('You are not allowed to remove the hold status.');
+            throw new AccessDeniedHttpException('only user can remove workflow on hold');
+        }
+
+        if (!$entityWorkflowStep->isOnHoldByUser($user)) {
+            throw new AccessDeniedHttpException('You are not allowed to remove workflow on hold');
+        }
+
+        $hold = $this->entityWorkflowStepHoldRepository->findOneByStepAndUser($entityWorkflowStep, $user);
+
+        if (null === $hold) {
+            // this should not happens...
+            throw new NotFoundHttpException();
         }
 
         $this->entityManager->remove($hold);
         $this->entityManager->flush();
 
-        return $this->redirectToRoute('chill_main_workflow_show', ['id' => $entityWorkflow->getId()]);
+        return $this->redirectToRoute('chill_main_workflow_show', ['id' => $entityWorkflowStep->getEntityWorkflow()->getId()]);
     }
 }

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

@@ -339,8 +339,6 @@ class EntityWorkflow implements TrackCreationInterface, TrackUpdateInterface
 
     public function isFreeze(): bool
     {
-        $steps = $this->getStepsChained();
-
         foreach ($this->getStepsChained() as $step) {
             if ($step->isFreezeAfter()) {
                 return true;
@@ -350,6 +348,11 @@ class EntityWorkflow implements TrackCreationInterface, TrackUpdateInterface
         return false;
     }
 
+    public function isOnHoldByUser(User $user): bool
+    {
+        return $this->getCurrentStep()->isOnHoldByUser($user);
+    }
+
     public function isUserSubscribedToFinal(User $user): bool
     {
         return $this->subscriberToFinal->contains($user);

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

@@ -286,6 +286,17 @@ class EntityWorkflowStep
         return $this->freezeAfter;
     }
 
+    public function isOnHoldByUser(User $user): bool
+    {
+        foreach ($this->getHoldsOnStep() as $onHold) {
+            if ($onHold->getByUser() === $user) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     public function isWaitingForTransition(): bool
     {
         if (null !== $this->transitionAfter) {

src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php

@@ -12,7 +12,6 @@ declare(strict_types=1);
 namespace Chill\MainBundle\Repository\Workflow;
 
 use Chill\MainBundle\Entity\User;
-use Chill\MainBundle\Entity\Workflow\EntityWorkflow;
 use Chill\MainBundle\Entity\Workflow\EntityWorkflowStep;
 use Chill\MainBundle\Entity\Workflow\EntityWorkflowStepHold;
 use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
@@ -58,23 +57,6 @@ class EntityWorkflowStepHoldRepository extends ServiceEntityRepository
         return $this->findBy(['step' => $step]);
     }
 
-    /**
-     * @throws NonUniqueResultException
-     */
-    public function findByWorkflow(EntityWorkflow $workflow): ?EntityWorkflowStepHold
-    {
-        $qb = $this->getEntityManager()->createQueryBuilder();
-
-        $qb->select('h')
-            ->from(EntityWorkflowStepHold::class, 'h')
-            ->join('h.step', 's')
-            ->join('s.entityWorkflow', 'w')
-            ->where('w = :workflow')
-            ->setParameter('workflow', $workflow);
-
-        return $qb->getQuery()->getOneOrNullResult();
-    }
-
     /**
      * Find a single EntityWorkflowStepHold by step and user.
      *

src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig

@@ -68,9 +68,9 @@
     <section class="step my-4">{% include '@ChillMain/Workflow/_history.html.twig' %}</section>
 
     <ul class="record_actions sticky-form-buttons">
-        {% if onHoldStep is not null  %}
+        {% if entity_workflow.isOnHoldByUser(app.user)  %}
             <li>
-                <a class="btn btn-misc" href="{{ path('chill_main_workflow_remove_hold', {'holdId': onHoldStep.id}) }}"><i class="fa fa-hourglass"></i>
+                <a class="btn btn-misc" href="{{ path('chill_main_workflow_remove_hold', {'id': entity_workflow.currentStep.id }) }}"><i class="fa fa-hourglass"></i>
                     {{ 'workflow.Remove hold'|trans }}
                 </a>
             </li>