From 49ad25b4c8f6869879b7786fd09fc998f4b4c933 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Fri, 6 Sep 2024 13:51:07 +0200 Subject: [PATCH] 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. --- .../Controller/WorkflowOnHoldController.php | 32 +++-- .../Entity/Workflow/EntityWorkflowStep.php | 9 ++ .../Workflow/EntityWorkflowStepHold.php | 5 +- .../WorkflowOnHoldControllerTest.php | 130 +++++++++--------- 4 files changed, 99 insertions(+), 77 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php index 6e4af7e22..08f62e53a 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php +++ b/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 EntityWorkflowRepository $entityWorkflowRepository, + private readonly UrlGeneratorInterface $urlGenerator, ) {} #[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()] + ) + ); } } diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php index 90aa632f8..c6a849da5 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php +++ b/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; + } } diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php index 3f4aeb669..3d163dfc5 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php +++ b/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 { diff --git a/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php b/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php index f4788c2ec..198adca5f 100644 --- a/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php +++ b/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\Repository\Workflow\EntityWorkflowStepHoldRepository; +use Chill\MainBundle\Workflow\EntityWorkflowMarkingStore; 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 $security; - private $registry; - private $entityWorkflowStepHoldRepository; - private $entityWorkflow; - private $workflow; - private $currentStep; - private $currentUser; - private $controller; - - protected function setUp(): void + private function buildRegistry(): Registry { - 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 - $this->entityManager = $this->createMock(EntityManagerInterface::class); - $this->security = $this->createMock(Security::class); - $this->registry = $this->createMock(Registry::class); - $this->entityWorkflowStepHoldRepository = $this->createMock(EntityWorkflowStepHoldRepository::class); - $this->entityWorkflowRepository = $this->createMock(EntityWorkflowRepository::class); + $workflow = new Workflow($definition, new EntityWorkflowMarkingStore(), name: 'dummy_workflow'); + $registry = new Registry(); + $registry->addWorkflow($workflow, new class () implements WorkflowSupportStrategyInterface { + public function supports(WorkflowInterface $workflow, object $subject): bool + { + return true; + } + }); - $this->entityWorkflow = $this->createMock(EntityWorkflow::class); - $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 - ); + return $registry; } public function testPutOnHoldPersistence(): void { - // Adjust mock for getEnabledTransitions to return a non-empty array - $transition = $this->createMock(Transition::class); - $transition->method('getName')->willReturn('dummy_transition'); + $entityWorkflow = new EntityWorkflow(); + $entityWorkflow->setWorkflowName('dummy_workflow'); + $security = $this->createMock(Security::class); + $security->method('getUser')->willReturn($user = new User()); - $this->workflow->method('getEnabledTransitions') - ->with($this->entityWorkflow) - ->willReturn([$transition]); - - $this->workflow->method('can') - ->with($this->entityWorkflow, 'dummy_transition') - ->willReturn(true); - - $this->entityManager->expects($this->once()) + $entityManager = $this->createMock(EntityManagerInterface::class); + $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(); - $response = $this->controller->putOnHold($this->entityWorkflow, $request); + $user = new User(); + $entityWorkflow = new EntityWorkflow(); + $entityWorkflow->setWorkflowName('dummy_workflow'); + $onHold = new EntityWorkflowStepHold($step = $entityWorkflow->getCurrentStep(), $user); - $this->assertInstanceOf(Response::class, $response); - $this->assertEquals(302, $response->getStatusCode()); + $security = $this->createMock(Security::class); + $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()); } }