Refactor WorkflowOnHoldController and improve tests

Refactored WorkflowOnHoldController to remove dependencies and improve redirect handling. Updated test cases to use PHPUnit instead of KernelTestCase and mock proper dependencies. Added relationship handling in EntityWorkflowStep to manage EntityWorkflowStepHold instances accurately.

src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php

@@ -15,33 +15,29 @@ 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;
 use Doctrine\ORM\EntityManagerInterface;
-use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\HttpFoundation\RedirectResponse;
 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\Routing\Generator\UrlGeneratorInterface;
 use Symfony\Component\Security\Core\Security;
 use Symfony\Component\Workflow\Registry;
 
-class WorkflowOnHoldController extends AbstractController
+class WorkflowOnHoldController
 {
     public function __construct(
         private readonly EntityManagerInterface $entityManager,
         private readonly Security $security,
         private readonly Registry $registry,
-        private readonly EntityWorkflowStepHoldRepository $entityWorkflowStepHoldRepository,
+        private readonly UrlGeneratorInterface $urlGenerator,
-        private readonly EntityWorkflowRepository $entityWorkflowRepository,
     ) {}
 
     #[Route(path: '/{_locale}/main/workflow/{id}/hold', name: 'chill_main_workflow_on_hold')]
     public function putOnHold(EntityWorkflow $entityWorkflow, Request $request): Response
     {
-        $entityWorkflow = $this->entityWorkflowRepository->find($entityWorkflow);
-
         $currentStep = $entityWorkflow->getCurrentStep();
         $currentUser = $this->security->getUser();
 
@@ -53,7 +49,7 @@ class WorkflowOnHoldController extends AbstractController
         $enabledTransitions = $workflow->getEnabledTransitions($entityWorkflow);
 
         if (0 === count($enabledTransitions)) {
-            throw $this->createAccessDeniedException('You are not allowed to apply any transitions to this workflow, therefore you cannot toggle the hold status.');
+            throw new AccessDeniedHttpException('You are not allowed to apply any transitions to this workflow, therefore you cannot toggle the hold status.');
         }
 
         $stepHold = new EntityWorkflowStepHold($currentStep, $currentUser);
@@ -61,11 +57,16 @@ class WorkflowOnHoldController extends AbstractController
         $this->entityManager->persist($stepHold);
         $this->entityManager->flush();
 
-        return $this->redirectToRoute('chill_main_workflow_show', ['id' => $entityWorkflow->getId()]);
+        return new RedirectResponse(
+            $this->urlGenerator->generate(
+                'chill_main_workflow_show',
+                ['id' => $entityWorkflow->getId()]
+            )
+        );
     }
 
     #[Route(path: '/{_locale}/main/workflow/{id}/remove_hold', name: 'chill_main_workflow_remove_hold')]
-    public function removeOnHold(EntityWorkflowStep $entityWorkflowStep, Request $request): Response
+    public function removeOnHold(EntityWorkflowStep $entityWorkflowStep): Response
     {
         $user = $this->security->getUser();
 
@@ -77,7 +78,7 @@ class WorkflowOnHoldController extends AbstractController
             throw new AccessDeniedHttpException('You are not allowed to remove workflow on hold');
         }
 
-        $hold = $this->entityWorkflowStepHoldRepository->findOneByStepAndUser($entityWorkflowStep, $user);
+        $hold = $entityWorkflowStep->getHoldsOnStep()->findFirst(fn(int $index, EntityWorkflowStepHold $entityWorkflowStepHold) => $user === $entityWorkflowStepHold->getByUser());
 
         if (null === $hold) {
             // this should not happens...
@@ -87,6 +88,11 @@ class WorkflowOnHoldController extends AbstractController
         $this->entityManager->remove($hold);
         $this->entityManager->flush();
 
-        return $this->redirectToRoute('chill_main_workflow_show', ['id' => $entityWorkflowStep->getEntityWorkflow()->getId()]);
+        return new RedirectResponse(
+            $this->urlGenerator->generate(
+                'chill_main_workflow_show',
+                ['id' => $entityWorkflowStep->getEntityWorkflow()->getId()]
+            )
+        );
     }
 }

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

@@ -455,4 +455,13 @@ class EntityWorkflowStep
             }
         }
     }
+
+    public function addOnHold(EntityWorkflowStepHold $onHold): self
+    {
+        if (!$this->holdsOnStep->contains($onHold)) {
+            $this->holdsOnStep->add($onHold);
+        }
+
+        return $this;
+    }
 }

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

@@ -32,7 +32,10 @@ class EntityWorkflowStepHold implements TrackCreationInterface
         #[ORM\JoinColumn(nullable: false)]
         private EntityWorkflowStep $step, #[ORM\ManyToOne(targetEntity: User::class)]
         #[ORM\JoinColumn(nullable: false)]
-        private User $byUser) {}
+        private User $byUser)
+    {
+        $step->addOnHold($this);
+    }
 
     public function getId(): ?int
     {

src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php

@@ -12,101 +12,105 @@ declare(strict_types=1);
 namespace Chill\MainBundle\Tests\Controller;
 
 use Chill\MainBundle\Controller\WorkflowOnHoldController;
+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\Workflow\EntityWorkflowMarkingStore;
-use Chill\MainBundle\Repository\Workflow\EntityWorkflowStepHoldRepository;
 use Doctrine\ORM\EntityManagerInterface;
-use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+use PHPUnit\Framework\TestCase;
 use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 use Symfony\Component\Security\Core\Security;
+use Symfony\Component\Workflow\DefinitionBuilder;
 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 WorkflowOnHoldControllerTest extends KernelTestCase
+class WorkflowOnHoldControllerTest extends TestCase
 {
-    private $entityManager;
+    private function buildRegistry(): Registry
-    private $security;
-    private $registry;
-    private $entityWorkflowStepHoldRepository;
-    private $entityWorkflow;
-    private $workflow;
-    private $currentStep;
-    private $currentUser;
-    private $controller;
-
-    protected function setUp(): void
     {
-        self::bootKernel();
+        $definitionBuilder = new DefinitionBuilder();
+        $definition = $definitionBuilder
+            ->addPlaces(['initial', 'layout', 'sign'])
+            ->addTransition(new Transition('to_layout', 'initial', 'layout'))
+            ->addTransition(new Transition('to_sign', 'initial', 'sign'))
+            ->build();
 
-        // Mocks
+        $workflow = new Workflow($definition, new EntityWorkflowMarkingStore(), name: 'dummy_workflow');
-        $this->entityManager = $this->createMock(EntityManagerInterface::class);
+        $registry = new Registry();
-        $this->security = $this->createMock(Security::class);
+        $registry->addWorkflow($workflow, new class () implements WorkflowSupportStrategyInterface {
-        $this->registry = $this->createMock(Registry::class);
+            public function supports(WorkflowInterface $workflow, object $subject): bool
-        $this->entityWorkflowStepHoldRepository = $this->createMock(EntityWorkflowStepHoldRepository::class);
+            {
-        $this->entityWorkflowRepository = $this->createMock(EntityWorkflowRepository::class);
+                return true;
+            }
+        });
 
-        $this->entityWorkflow = $this->createMock(EntityWorkflow::class);
+        return $registry;
-        $this->workflow = $this->createMock(Workflow::class);
-        $this->currentStep = $this->createMock(EntityWorkflowStep::class);
-        $this->currentUser = $this->createMock(\Chill\MainBundle\Entity\User::class);
-
-        // Define expected behaviors for the mocks
-        $this->entityWorkflow->method('getCurrentStep')->willReturn($this->currentStep);
-        $this->security->method('getUser')->willReturn($this->currentUser);
-        $this->entityWorkflow->method('getWorkflowName')->willReturn('workflow_name');
-        $this->registry->method('get')->with($this->entityWorkflow, 'workflow_name')->willReturn($this->workflow);
-        $this->workflow->method('getEnabledTransitions')->with($this->entityWorkflow)->willReturn([]);
-        $this->entityWorkflow->method('getUsersInvolved')->willReturn([]);
-
-        $this->controller = new WorkflowOnHoldController(
-            $this->entityManager,
-            $this->security,
-            $this->registry,
-            $this->entityWorkflowStepHoldRepository,
-            $this->entityWorkflowRepository
-        );
     }
 
     public function testPutOnHoldPersistence(): void
     {
-        // Adjust mock for getEnabledTransitions to return a non-empty array
+        $entityWorkflow = new EntityWorkflow();
-        $transition = $this->createMock(Transition::class);
+        $entityWorkflow->setWorkflowName('dummy_workflow');
-        $transition->method('getName')->willReturn('dummy_transition');
+        $security = $this->createMock(Security::class);
+        $security->method('getUser')->willReturn($user = new User());
 
-        $this->workflow->method('getEnabledTransitions')
+        $entityManager = $this->createMock(EntityManagerInterface::class);
-            ->with($this->entityWorkflow)
+        $entityManager->expects($this->once())
-            ->willReturn([$transition]);
-
-        $this->workflow->method('can')
-            ->with($this->entityWorkflow, 'dummy_transition')
-            ->willReturn(true);
-
-        $this->entityManager->expects($this->once())
             ->method('persist')
             ->with($this->isInstanceOf(EntityWorkflowStepHold::class));
 
-        $this->entityManager->expects($this->once())
+        $entityManager->expects($this->once())
             ->method('flush');
 
+        $urlGenerator = $this->createMock(UrlGeneratorInterface::class);
+        $urlGenerator->method('generate')
+            ->with('chill_main_workflow_show', ['id' => null])
+            ->willReturn('/some/url');
+
+        $controller = new WorkflowOnHoldController($entityManager, $security, $this->buildRegistry(), $urlGenerator);
+
         $request = new Request();
-        $this->controller->putOnHold($this->entityWorkflow, $request);
+        $response = $controller->putOnHold($entityWorkflow, $request);
+
+        self::assertEquals(302, $response->getStatusCode());
     }
 
-    public function testPutOnHoldSmokeTest(): void
+    public function testRemoveOnHold(): void
     {
-        $request = new Request();
+        $user = new User();
-        $response = $this->controller->putOnHold($this->entityWorkflow, $request);
+        $entityWorkflow = new EntityWorkflow();
+        $entityWorkflow->setWorkflowName('dummy_workflow');
+        $onHold = new EntityWorkflowStepHold($step = $entityWorkflow->getCurrentStep(), $user);
 
-        $this->assertInstanceOf(Response::class, $response);
+        $security = $this->createMock(Security::class);
-        $this->assertEquals(302, $response->getStatusCode());
+        $security->method('getUser')->willReturn($user);
+
+        $entityManager = $this->createMock(EntityManagerInterface::class);
+        $entityManager->expects($this->once())
+            ->method('remove')
+            ->with($onHold);
+
+        $entityManager->expects($this->once())
+            ->method('flush');
+
+        $urlGenerator = $this->createMock(UrlGeneratorInterface::class);
+        $urlGenerator->method('generate')
+            ->with('chill_main_workflow_show', ['id' => null])
+            ->willReturn('/some/url');
+
+        $controller = new WorkflowOnHoldController($entityManager, $security, $this->buildRegistry(), $urlGenerator);
+
+        $response = $controller->removeOnHold($step);
+
+        self::assertEquals(302, $response->getStatusCode());
     }
 }