From 9475a708c39bc4f955947bc27b559c080c5bd2e5 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 7 Aug 2024 16:47:29 +0200 Subject: [PATCH 01/21] Add EntityWorkflowStepHold entity to allow workflow to be put on hold by user Entity created, migration, and repository. --- .../Workflow/EntityWorkflowStepHold.php | 60 +++++++++++++++ .../EntityWorkflowStepHoldRepository.php | 74 +++++++++++++++++++ .../migrations/Version20240807123801.php | 41 ++++++++++ 3 files changed, 175 insertions(+) create mode 100644 src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php create mode 100644 src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php create mode 100644 src/Bundle/ChillMainBundle/migrations/Version20240807123801.php diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php new file mode 100644 index 000000000..6005b284f --- /dev/null +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php @@ -0,0 +1,60 @@ +id; + } + + public function setId(?int $id): void + { + $this->id = $id; + } + + public function getStep(): EntityWorkflowStep + { + return $this->step; + } + + public function setStep(EntityWorkflowStep $step): void + { + $this->step = $step; + } + + public function getByUser(): User + { + return $this->byUser; + } + + public function setByUser(User $byUser): void + { + $this->byUser = $byUser; + } + +} diff --git a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php new file mode 100644 index 000000000..59f0464f8 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php @@ -0,0 +1,74 @@ +find($id); + } + + /** + * Find all EntityWorkflowStepHold records. + * + * @return EntityWorkflowStepHold[] + */ + public function findAllHolds(): array + { + return $this->findAll(); + } + + /** + * Find EntityWorkflowStepHold by a specific step. + * + * @param EntityWorkflowStep $step + * @return EntityWorkflowStepHold[] + */ + public function findByStep(EntityWorkflowStep $step): array + { + return $this->findBy(['step' => $step]); + } + + /** + * Find a single EntityWorkflowStepHold by step and user. + * + * @param EntityWorkflowStep $step + * @param User $user + * @return EntityWorkflowStepHold|null + * @throws NonUniqueResultException + */ + public function findOneByStepAndUser(EntityWorkflowStep $step, User $user): ?EntityWorkflowStepHold + { + try { + return $this->createQueryBuilder('e') + ->andWhere('e.step = :step') + ->andWhere('e.byUser = :user') + ->setParameter('step', $step) + ->setParameter('user', $user) + ->getQuery() + ->getSingleResult(); + } catch (NoResultException $e) { + return null; + } + } +} diff --git a/src/Bundle/ChillMainBundle/migrations/Version20240807123801.php b/src/Bundle/ChillMainBundle/migrations/Version20240807123801.php new file mode 100644 index 000000000..e626778e9 --- /dev/null +++ b/src/Bundle/ChillMainBundle/migrations/Version20240807123801.php @@ -0,0 +1,41 @@ +addSql('CREATE SEQUENCE chill_main_workflow_entity_step_hold_id_seq INCREMENT BY 1 MINVALUE 1 START 1'); + $this->addSql('CREATE TABLE chill_main_workflow_entity_step_hold (id INT NOT NULL, step_id INT NOT NULL, createdAt TIMESTAMP(0) WITHOUT TIME ZONE DEFAULT NULL, byUser_id INT NOT NULL, createdBy_id INT DEFAULT NULL, PRIMARY KEY(id))'); + $this->addSql('CREATE INDEX IDX_1BE2E7C73B21E9C ON chill_main_workflow_entity_step_hold (step_id)'); + $this->addSql('CREATE INDEX IDX_1BE2E7CD23C0240 ON chill_main_workflow_entity_step_hold (byUser_id)'); + $this->addSql('CREATE INDEX IDX_1BE2E7C3174800F ON chill_main_workflow_entity_step_hold (createdBy_id)'); + $this->addSql('COMMENT ON COLUMN chill_main_workflow_entity_step_hold.createdAt IS \'(DC2Type:datetime_immutable)\''); + $this->addSql('ALTER TABLE chill_main_workflow_entity_step_hold ADD CONSTRAINT FK_1BE2E7C73B21E9C FOREIGN KEY (step_id) REFERENCES chill_main_workflow_entity_step (id) NOT DEFERRABLE INITIALLY IMMEDIATE'); + $this->addSql('ALTER TABLE chill_main_workflow_entity_step_hold ADD CONSTRAINT FK_1BE2E7CD23C0240 FOREIGN KEY (byUser_id) REFERENCES users (id) NOT DEFERRABLE INITIALLY IMMEDIATE'); + $this->addSql('ALTER TABLE chill_main_workflow_entity_step_hold ADD CONSTRAINT FK_1BE2E7C3174800F FOREIGN KEY (createdBy_id) REFERENCES users (id) NOT DEFERRABLE INITIALLY IMMEDIATE'); + } + + public function down(Schema $schema): void + { + $this->addSql('DROP SEQUENCE chill_main_workflow_entity_step_hold_id_seq CASCADE'); + $this->addSql('ALTER TABLE chill_main_workflow_entity_step_hold DROP CONSTRAINT FK_1BE2E7C73B21E9C'); + $this->addSql('ALTER TABLE chill_main_workflow_entity_step_hold DROP CONSTRAINT FK_1BE2E7CD23C0240'); + $this->addSql('ALTER TABLE chill_main_workflow_entity_step_hold DROP CONSTRAINT FK_1BE2E7C3174800F'); + $this->addSql('DROP TABLE chill_main_workflow_entity_step_hold'); + } +} From 42471269db32b721217dc302b8a81283f19b2c38 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 7 Aug 2024 16:48:59 +0200 Subject: [PATCH 02/21] Implement logic to put workflow on hold and resume it --- .../Controller/WorkflowController.php | 8 +++++++ .../Resources/views/Workflow/index.html.twig | 21 ++++++++++++------- .../translations/messages.fr.yml | 2 ++ 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php index eb72c2624..71ff897e0 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php @@ -19,11 +19,13 @@ 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; use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO; use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\NonUniqueResultException; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\Clock\ClockInterface; use Symfony\Component\Form\Extension\Core\Type\FormType; @@ -52,6 +54,7 @@ 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, ) {} @@ -283,6 +286,9 @@ class WorkflowController extends AbstractController ); } + /** + * @throws NonUniqueResultException + */ #[Route(path: '/{_locale}/main/workflow/{id}/show', name: 'chill_main_workflow_show')] public function show(EntityWorkflow $entityWorkflow, Request $request): Response { @@ -292,6 +298,7 @@ class WorkflowController extends AbstractController $workflow = $this->registry->get($entityWorkflow, $entityWorkflow->getWorkflowName()); $errors = []; $signatures = $entityWorkflow->getCurrentStep()->getSignatures(); + $holdOnStepByUser = $this->entityWorkflowStepHoldRepository->findOneByStepAndUser($entityWorkflow->getCurrentStep(), $this->security->getUser()); if (\count($workflow->getEnabledTransitions($entityWorkflow)) > 0) { // possible transition @@ -354,6 +361,7 @@ class WorkflowController extends AbstractController 'entity_workflow' => $entityWorkflow, 'transition_form_errors' => $errors, 'signatures' => $signatures, + 'holdOnStepByUser' => $holdOnStepByUser, ] ); } diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig index da4d073b2..5955f89c5 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig @@ -64,14 +64,21 @@
{% include '@ChillMain/Workflow/_comment.html.twig' %}
#}
{% include '@ChillMain/Workflow/_history.html.twig' %}
- {# useful ? - #} + {% endblock %} diff --git a/src/Bundle/ChillMainBundle/translations/messages.fr.yml b/src/Bundle/ChillMainBundle/translations/messages.fr.yml index f6f30bcd0..335de609d 100644 --- a/src/Bundle/ChillMainBundle/translations/messages.fr.yml +++ b/src/Bundle/ChillMainBundle/translations/messages.fr.yml @@ -527,6 +527,8 @@ workflow: Access link copied: Lien d'accès copié This link grant any user to apply a transition: Le lien d'accès suivant permet d'appliquer une transition The workflow may be accssed through this link: Une transition peut être appliquée sur ce workflow grâce au lien d'accès suivant + Put on hold: Mettre en attente + Remove hold: Enlever la mise en attente signature_zone: title: Appliquer les signatures électroniques From 15eaf648df384763b0684199162a3317e5a82954 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 7 Aug 2024 17:25:41 +0200 Subject: [PATCH 03/21] Add label to workflow listing and history to indicate whether it's on hold --- .../ChillMainBundle/Entity/Workflow/EntityWorkflow.php | 5 +++++ .../Entity/Workflow/EntityWorkflowStep.php | 9 +++++++++ .../Resources/views/Workflow/_history.html.twig | 6 +++++- .../Resources/views/Workflow/list.html.twig | 3 +++ src/Bundle/ChillMainBundle/translations/messages.fr.yml | 1 + 5 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php index 5a3359c46..14b39a065 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php @@ -480,4 +480,9 @@ class EntityWorkflow implements TrackCreationInterface, TrackUpdateInterface return $this->steps->get($this->steps->count() - 2); } + + public function isOnHoldAtCurrentStep(): bool + { + return $this->getCurrentStep()->getHoldsOnStep()->count() > 0; + } } diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php index 953fb31b1..156588bd2 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php @@ -98,12 +98,16 @@ class EntityWorkflowStep #[ORM\Column(type: \Doctrine\DBAL\Types\Types::TEXT, nullable: true)] private ?string $transitionByEmail = null; + #[ORM\OneToMany(mappedBy: 'step', targetEntity: EntityWorkflowStepHold::class)] + private Collection $holdsOnStep; + public function __construct() { $this->ccUser = new ArrayCollection(); $this->destUser = new ArrayCollection(); $this->destUserByAccessKey = new ArrayCollection(); $this->signatures = new ArrayCollection(); + $this->holdsOnStep = new ArrayCollection(); $this->accessKey = bin2hex(openssl_random_pseudo_bytes(32)); } @@ -413,6 +417,11 @@ class EntityWorkflowStep return $this; } + public function getHoldsOnStep(): Collection + { + return $this->holdsOnStep; + } + #[Assert\Callback] public function validateOnCreation(ExecutionContextInterface $context, mixed $payload): void { diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_history.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_history.html.twig index a3e2e24b9..a56656102 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_history.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_history.html.twig @@ -76,7 +76,11 @@

{{ 'workflow.Users allowed to apply transition'|trans }} :

    {% for u in step.destUser %} -
  • {{ u|chill_entity_render_box({'at_date': step.previous.transitionAt}) }}
  • +
  • {{ u|chill_entity_render_box({'at_date': step.previous.transitionAt}) }} + {% if entity_workflow.isOnHoldAtCurrentStep %} + {{ 'workflow.On hold'|trans }} + {% endif %} +
  • {% endfor %}
{% endif %} diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/list.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/list.html.twig index 5d8c39c63..5dd6dc714 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/list.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/list.html.twig @@ -69,6 +69,9 @@
{{ macro.breadcrumb(l) }} + {% if l.entity_workflow.isOnHoldAtCurrentStep %} + {{ 'workflow.On hold'|trans }} + {% endif %}
diff --git a/src/Bundle/ChillMainBundle/translations/messages.fr.yml b/src/Bundle/ChillMainBundle/translations/messages.fr.yml index 335de609d..f5c2cfc46 100644 --- a/src/Bundle/ChillMainBundle/translations/messages.fr.yml +++ b/src/Bundle/ChillMainBundle/translations/messages.fr.yml @@ -529,6 +529,7 @@ workflow: The workflow may be accssed through this link: Une transition peut être appliquée sur ce workflow grâce au lien d'accès suivant Put on hold: Mettre en attente Remove hold: Enlever la mise en attente + On hold: En attente signature_zone: title: Appliquer les signatures électroniques From c21de777fd1f8bac5e1958039950de43107adca8 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Thu, 8 Aug 2024 12:18:59 +0200 Subject: [PATCH 04/21] Place on_hold label next to breadcrumb in twig template --- .../ChillMainBundle/Resources/views/Workflow/index.html.twig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig index 5955f89c5..191499b0a 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig @@ -39,6 +39,9 @@

{{ handler.entityTitle(entity_workflow) }}

{{ macro.breadcrumb({'entity_workflow': entity_workflow}) }} + {% if entity_workflow.isOnHoldAtCurrentStep %} + {{ 'workflow.On hold'|trans }} + {% endif %} {% include handler_template with handler_template_data|merge({'display_action': true }) %} From d119ba49f70cff54bd16bde29b568b70c128b3d4 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Thu, 8 Aug 2024 12:19:33 +0200 Subject: [PATCH 05/21] Add on_hold label to vue components --- .../ChillMainBundle/Resources/public/chill/chillmain.scss | 1 + .../public/vuejs/HomepageWidget/MyWorkflowsTable.vue | 5 ++++- .../Resources/public/vuejs/HomepageWidget/js/i18n.js | 1 + .../vuejs/_components/EntityWorkflow/ListWorkflow.vue | 6 ++++-- .../Serializer/Normalizer/EntityWorkflowNormalizer.php | 1 + 5 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Resources/public/chill/chillmain.scss b/src/Bundle/ChillMainBundle/Resources/public/chill/chillmain.scss index eabc2adc4..4f215859f 100644 --- a/src/Bundle/ChillMainBundle/Resources/public/chill/chillmain.scss +++ b/src/Bundle/ChillMainBundle/Resources/public/chill/chillmain.scss @@ -498,6 +498,7 @@ div.workflow { div.breadcrumb { display: initial; margin-bottom: 0; + margin-right: .5rem; padding-right: 0.5em; background-color: tint-color($chill-yellow, 90%); border: 1px solid $chill-yellow; diff --git a/src/Bundle/ChillMainBundle/Resources/public/vuejs/HomepageWidget/MyWorkflowsTable.vue b/src/Bundle/ChillMainBundle/Resources/public/vuejs/HomepageWidget/MyWorkflowsTable.vue index f98d7a5cb..f82e8a88c 100644 --- a/src/Bundle/ChillMainBundle/Resources/public/vuejs/HomepageWidget/MyWorkflowsTable.vue +++ b/src/Bundle/ChillMainBundle/Resources/public/vuejs/HomepageWidget/MyWorkflowsTable.vue @@ -9,13 +9,16 @@ + {{ $t('on_hold') }}
@@ -73,7 +74,8 @@ const i18n = { you_subscribed_to_all_steps: "Vous recevrez une notification à chaque étape", you_subscribed_to_final_step: "Vous recevrez une notification à l'étape finale", by: "Par", - at: "Le" + at: "Le", + on_hold: "En attente" } } } diff --git a/src/Bundle/ChillMainBundle/Serializer/Normalizer/EntityWorkflowNormalizer.php b/src/Bundle/ChillMainBundle/Serializer/Normalizer/EntityWorkflowNormalizer.php index b1f3807ed..a84220099 100644 --- a/src/Bundle/ChillMainBundle/Serializer/Normalizer/EntityWorkflowNormalizer.php +++ b/src/Bundle/ChillMainBundle/Serializer/Normalizer/EntityWorkflowNormalizer.php @@ -45,6 +45,7 @@ class EntityWorkflowNormalizer implements NormalizerInterface, NormalizerAwareIn 'steps' => $this->normalizer->normalize($object->getStepsChained(), $format, $context), 'datas' => $this->normalizer->normalize($handler->getEntityData($object), $format, $context), 'title' => $handler->getEntityTitle($object), + 'isOnHoldAtCurrentStep' => $object->isOnHoldAtCurrentStep(), ]; } From e85c31826f264454f8d10bde7b102d2a184a9e11 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Thu, 8 Aug 2024 12:20:41 +0200 Subject: [PATCH 06/21] php cs fixes --- .../Entity/Workflow/EntityWorkflowStepHold.php | 11 +++++++++-- .../EntityWorkflowStepHoldRepository.php | 16 +++++++++------- .../migrations/Version20240807123801.php | 7 +++++++ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php index 6005b284f..166b8cfd2 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php @@ -1,5 +1,14 @@ byUser = $byUser; } - } diff --git a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php index 59f0464f8..f445b33ee 100644 --- a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php +++ b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php @@ -1,5 +1,14 @@ Date: Thu, 8 Aug 2024 13:00:09 +0200 Subject: [PATCH 07/21] Rector fixes --- .../ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php index 156588bd2..1cdb157b1 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php @@ -98,6 +98,9 @@ class EntityWorkflowStep #[ORM\Column(type: \Doctrine\DBAL\Types\Types::TEXT, nullable: true)] private ?string $transitionByEmail = null; + /** + * @var \Doctrine\Common\Collections\Collection + */ #[ORM\OneToMany(mappedBy: 'step', targetEntity: EntityWorkflowStepHold::class)] private Collection $holdsOnStep; From 6fc5a10dc4dd24caa5284c40743ce01a2a27f287 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 28 Aug 2024 17:15:09 +0200 Subject: [PATCH 08/21] Add unique constraint to migration --- .../ChillMainBundle/migrations/Version20240807123801.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Bundle/ChillMainBundle/migrations/Version20240807123801.php b/src/Bundle/ChillMainBundle/migrations/Version20240807123801.php index 746066cc8..1e95bd359 100644 --- a/src/Bundle/ChillMainBundle/migrations/Version20240807123801.php +++ b/src/Bundle/ChillMainBundle/migrations/Version20240807123801.php @@ -14,9 +14,6 @@ namespace Chill\Migrations\Main; use Doctrine\DBAL\Schema\Schema; use Doctrine\Migrations\AbstractMigration; -/** - * Auto-generated Migration: Please modify to your needs! - */ final class Version20240807123801 extends AbstractMigration { public function getDescription(): string @@ -31,6 +28,7 @@ final class Version20240807123801 extends AbstractMigration $this->addSql('CREATE INDEX IDX_1BE2E7C73B21E9C ON chill_main_workflow_entity_step_hold (step_id)'); $this->addSql('CREATE INDEX IDX_1BE2E7CD23C0240 ON chill_main_workflow_entity_step_hold (byUser_id)'); $this->addSql('CREATE INDEX IDX_1BE2E7C3174800F ON chill_main_workflow_entity_step_hold (createdBy_id)'); + $this->addSql('CREATE UNIQUE INDEX chill_main_workflow_hold_unique_idx ON chill_main_workflow_entity_step_hold (step_id, byUser_id)'); $this->addSql('COMMENT ON COLUMN chill_main_workflow_entity_step_hold.createdAt IS \'(DC2Type:datetime_immutable)\''); $this->addSql('ALTER TABLE chill_main_workflow_entity_step_hold ADD CONSTRAINT FK_1BE2E7C73B21E9C FOREIGN KEY (step_id) REFERENCES chill_main_workflow_entity_step (id) NOT DEFERRABLE INITIALLY IMMEDIATE'); $this->addSql('ALTER TABLE chill_main_workflow_entity_step_hold ADD CONSTRAINT FK_1BE2E7CD23C0240 FOREIGN KEY (byUser_id) REFERENCES users (id) NOT DEFERRABLE INITIALLY IMMEDIATE'); From c1e5f4a57e15bd0ac297a41a4c3d4f0035850079 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 28 Aug 2024 17:15:38 +0200 Subject: [PATCH 09/21] Move onHold methods to a separate WorkflowOnHold controller --- .../Controller/WorkflowOnHoldController.php | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php new file mode 100644 index 000000000..4d86dfb0a --- /dev/null +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php @@ -0,0 +1,59 @@ +getCurrentStep(); + $currentUser = $this->security->getUser(); + + $workflow = $this->registry->get($entityWorkflow, $entityWorkflow->getWorkflowName()); + + $enabledTransitions = $workflow->getEnabledTransitions($entityWorkflow); + $usersInvolved = $entityWorkflow->getUsersInvolved(); + + /* if (count($enabledTransitions) > 0) { + + }*/ + + $stepHold = new EntityWorkflowStepHold($currentStep, $currentUser); + + $this->entityManager->persist($stepHold); + $this->entityManager->flush(); + + return $this->redirectToRoute('chill_main_workflow_show', ['id' => $entityWorkflow->getId()]); + } + + #[Route(path: '/{_locale}/main/workflow/{holdId}/remove_hold', name: 'chill_main_workflow_remove_hold')] + public function removeOnHold(int $holdId, Request $request): Response + { + $hold = $this->entityWorkflowStepHoldRepository->findById($holdId); + $entityWorkflow = $hold->getStep()->getEntityWorkflow(); + + $this->entityManager->remove($hold); + $this->entityManager->flush(); + + return $this->redirectToRoute('chill_main_workflow_show', ['id' => $entityWorkflow->getId()]); + } +} From deb4bda16efd74d61adbebf5a51ae859df64839e Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 28 Aug 2024 17:16:50 +0200 Subject: [PATCH 10/21] Add unique constraint as attribute on EntityWorkflowStepHold class --- .../Workflow/EntityWorkflowStepHold.php | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php index 166b8cfd2..47ae0f954 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php @@ -18,6 +18,7 @@ use Doctrine\ORM\Mapping as ORM; #[ORM\Entity] #[ORM\Table('chill_main_workflow_entity_step_hold')] +#[ORM\UniqueConstraint(name: 'chill_main_workflow_hold_unique_idx', columns: ['step_id', 'byUser_id'])] class EntityWorkflowStepHold implements TrackCreationInterface { use TrackCreationTrait; @@ -35,33 +36,24 @@ class EntityWorkflowStepHold implements TrackCreationInterface #[ORM\JoinColumn(nullable: false)] private User $byUser; + public function __construct(EntityWorkflowStep $step, User $byUser) + { + $this->step = $step; + $this->byUser = $byUser; + } + public function getId(): ?int { return $this->id; } - public function setId(?int $id): void - { - $this->id = $id; - } - public function getStep(): EntityWorkflowStep { return $this->step; } - public function setStep(EntityWorkflowStep $step): void - { - $this->step = $step; - } - public function getByUser(): User { return $this->byUser; } - - public function setByUser(User $byUser): void - { - $this->byUser = $byUser; - } } From a82b99aeccda4ad30f65a2ad717131938319140c Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 28 Aug 2024 17:17:24 +0200 Subject: [PATCH 11/21] Add template-extends on EntityWorkflowStepHoldRepository --- .../Repository/Workflow/EntityWorkflowStepHoldRepository.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php index f445b33ee..9e1316e05 100644 --- a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php +++ b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php @@ -19,6 +19,10 @@ use Doctrine\ORM\NonUniqueResultException; use Doctrine\ORM\NoResultException; use Doctrine\Persistence\ManagerRegistry; +/** + * @template-extends ServiceEntityRepository + */ + class EntityWorkflowStepHoldRepository extends ServiceEntityRepository { public function __construct(ManagerRegistry $registry) From 9c8a84cdbd4525006b07ee7f7935269ec0b64431 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 28 Aug 2024 17:26:58 +0200 Subject: [PATCH 12/21] Add test for WorkflowOnHoldControllerTest --- .../WorkflowOnHoldControllerTest.php | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php diff --git a/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php b/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php new file mode 100644 index 000000000..b603c0f25 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php @@ -0,0 +1,61 @@ +createMock(EntityManagerInterface::class); + $security = $this->createMock(ChillSecurity::class); + $registry = $this->createMock(Registry::class); + $entityWorkflowStepHoldRepository = $this->createMock(EntityWorkflowStepHoldRepository::class); + $entityWorkflow = $this->createMock(EntityWorkflow::class); + $workflow = $this->createMock(Workflow::class); + $request = $this->createMock(Request::class); + $currentStep = $this->createMock(EntityWorkflow::class); // Assuming EntityWorkflow is the type, adjust if needed + $currentUser = $this->createMock(\Chill\MainBundle\Entity\User::class); // Adjust with actual User entity class + + // Define expected behaviors for the mocks + $entityWorkflow->method('getCurrentStep')->willReturn($currentStep); + $security->method('getUser')->willReturn($currentUser); + $entityWorkflow->method('getWorkflowName')->willReturn('workflow_name'); + $registry->method('get')->with($entityWorkflow, 'workflow_name')->willReturn($workflow); + $workflow->method('getEnabledTransitions')->with($entityWorkflow)->willReturn([]); + $entityWorkflow->method('getUsersInvolved')->willReturn([]); + + $entityManager->expects($this->once()) + ->method('persist') + ->with($this->isInstanceOf(EntityWorkflowStepHold::class)); + + $entityManager->expects($this->once()) + ->method('flush'); + + $controller = new WorkflowOnHoldController( + $entityManager, + $security, + $registry, + $entityWorkflowStepHoldRepository + ); + + // Smoke test + $response = $controller->putOnHold($entityWorkflow, $request); + + $this->assertInstanceOf(Response::class, $response); + $this->assertEquals(302, $response->getStatusCode()); + } +} From 8c5e94e29571a4bc2fea62dc3e92b50f1ae38b1f Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Thu, 29 Aug 2024 16:10:23 +0200 Subject: [PATCH 13/21] Work on test for workflowOnHold controller --- .../Controller/WorkflowOnHoldController.php | 3 ++- .../WorkflowOnHoldControllerTest.php | 21 ++++++++++++------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php index 4d86dfb0a..5698fe799 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php @@ -11,13 +11,14 @@ use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Annotation\Route; +use Symfony\Component\Security\Core\Security; use Symfony\Component\Workflow\Registry; class WorkflowOnHoldController extends AbstractController { public function __construct( private readonly EntityManagerInterface $entityManager, - private readonly ChillSecurity $security, + private readonly Security $security, private readonly Registry $registry, private readonly EntityWorkflowStepHoldRepository $entityWorkflowStepHoldRepository ) {} diff --git a/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php b/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php index b603c0f25..db9e0cc64 100644 --- a/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php @@ -4,31 +4,38 @@ namespace Chill\MainBundle\Tests\Controller; use Chill\MainBundle\Controller\WorkflowOnHoldController; use Chill\MainBundle\Entity\Workflow\EntityWorkflow; +use Chill\MainBundle\Entity\Workflow\EntityWorkflowStep; use Chill\MainBundle\Entity\Workflow\EntityWorkflowStepHold; use Chill\MainBundle\Repository\Workflow\EntityWorkflowStepHoldRepository; use Chill\MainBundle\Security\ChillSecurity; use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\TestCase; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use Symfony\Component\DependencyInjection\ContainerInterface; 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\Registry; use Symfony\Component\Workflow\Workflow; -class WorkflowOnHoldControllerTest extends TestCase +class WorkflowOnHoldControllerTest extends KernelTestCase { public function testPutOnHoldSuccess() { + self::bootKernel(); + // Mocks $entityManager = $this->createMock(EntityManagerInterface::class); - $security = $this->createMock(ChillSecurity::class); + $security = $this->createMock(Security::class); $registry = $this->createMock(Registry::class); $entityWorkflowStepHoldRepository = $this->createMock(EntityWorkflowStepHoldRepository::class); + $entityWorkflow = $this->createMock(EntityWorkflow::class); $workflow = $this->createMock(Workflow::class); - $request = $this->createMock(Request::class); - $currentStep = $this->createMock(EntityWorkflow::class); // Assuming EntityWorkflow is the type, adjust if needed - $currentUser = $this->createMock(\Chill\MainBundle\Entity\User::class); // Adjust with actual User entity class + $request = new Request(); + $currentStep = $this->createMock(EntityWorkflowStep::class); + $currentUser = $this->createMock(\Chill\MainBundle\Entity\User::class); // Define expected behaviors for the mocks $entityWorkflow->method('getCurrentStep')->willReturn($currentStep); @@ -52,10 +59,10 @@ class WorkflowOnHoldControllerTest extends TestCase $entityWorkflowStepHoldRepository ); - // Smoke test +/* // Smoke test $response = $controller->putOnHold($entityWorkflow, $request); $this->assertInstanceOf(Response::class, $response); - $this->assertEquals(302, $response->getStatusCode()); + $this->assertEquals(302, $response->getStatusCode());*/ } } From 46b31ae1ea204dfc666037ad737c40fae49a478a Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Fri, 30 Aug 2024 11:29:25 +0200 Subject: [PATCH 14/21] Adjust logic to only allow on hold if user is allowed to apply a transition --- .../Controller/WorkflowOnHoldController.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php index 5698fe799..a7831159c 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php @@ -32,11 +32,21 @@ class WorkflowOnHoldController extends AbstractController $workflow = $this->registry->get($entityWorkflow, $entityWorkflow->getWorkflowName()); $enabledTransitions = $workflow->getEnabledTransitions($entityWorkflow); - $usersInvolved = $entityWorkflow->getUsersInvolved(); + if (\count($enabledTransitions) === 0) { + throw $this->createAccessDeniedException('No transitions are available for the current workflow state.'); + } - /* if (count($enabledTransitions) > 0) { + $isTransitionAllowed = false; + foreach ($enabledTransitions as $transition) { + if ($workflow->can($entityWorkflow, $transition->getName())) { + $isTransitionAllowed = true; + break; + } + } - }*/ + if (!$isTransitionAllowed) { + throw $this->createAccessDeniedException('You are not allowed to apply any transitions to this workflow, therefore you cannot put it on hold.'); + } $stepHold = new EntityWorkflowStepHold($currentStep, $currentUser); From 41ffc470a0cf9caa167d832c0a69a316ed441c94 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Fri, 30 Aug 2024 11:30:06 +0200 Subject: [PATCH 15/21] Adjust test to check creation of onHold if user is allowed to apply a transition --- .../WorkflowOnHoldControllerTest.php | 87 +++++++++++++------ 1 file changed, 62 insertions(+), 25 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php b/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php index db9e0cc64..785d5d33c 100644 --- a/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php @@ -17,52 +17,89 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Security\Core\Security; use Symfony\Component\Workflow\Registry; +use Symfony\Component\Workflow\Transition; use Symfony\Component\Workflow\Workflow; class WorkflowOnHoldControllerTest extends KernelTestCase { - public function testPutOnHoldSuccess() + private $entityManager; + private $security; + private $registry; + private $entityWorkflowStepHoldRepository; + private $entityWorkflow; + private $workflow; + private $currentStep; + private $currentUser; + + protected function setUp(): void { self::bootKernel(); // Mocks - $entityManager = $this->createMock(EntityManagerInterface::class); - $security = $this->createMock(Security::class); - $registry = $this->createMock(Registry::class); - $entityWorkflowStepHoldRepository = $this->createMock(EntityWorkflowStepHoldRepository::class); + $this->entityManager = $this->createMock(EntityManagerInterface::class); + $this->security = $this->createMock(Security::class); + $this->registry = $this->createMock(Registry::class); + $this->entityWorkflowStepHoldRepository = $this->createMock(EntityWorkflowStepHoldRepository::class); - $entityWorkflow = $this->createMock(EntityWorkflow::class); - $workflow = $this->createMock(Workflow::class); - $request = new Request(); - $currentStep = $this->createMock(EntityWorkflowStep::class); - $currentUser = $this->createMock(\Chill\MainBundle\Entity\User::class); + $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 - $entityWorkflow->method('getCurrentStep')->willReturn($currentStep); - $security->method('getUser')->willReturn($currentUser); - $entityWorkflow->method('getWorkflowName')->willReturn('workflow_name'); - $registry->method('get')->with($entityWorkflow, 'workflow_name')->willReturn($workflow); - $workflow->method('getEnabledTransitions')->with($entityWorkflow)->willReturn([]); - $entityWorkflow->method('getUsersInvolved')->willReturn([]); + $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([]); + } - $entityManager->expects($this->once()) + 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'); + + $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()) ->method('persist') ->with($this->isInstanceOf(EntityWorkflowStepHold::class)); - $entityManager->expects($this->once()) + $this->entityManager->expects($this->once()) ->method('flush'); $controller = new WorkflowOnHoldController( - $entityManager, - $security, - $registry, - $entityWorkflowStepHoldRepository + $this->entityManager, + $this->security, + $this->registry, + $this->entityWorkflowStepHoldRepository ); -/* // Smoke test - $response = $controller->putOnHold($entityWorkflow, $request); + $request = new Request(); + $controller->putOnHold($this->entityWorkflow, $request); + } + + public function testPutOnHoldSmokeTest(): void + { + $controller = new WorkflowOnHoldController( + $this->entityManager, + $this->security, + $this->registry, + $this->entityWorkflowStepHoldRepository + ); + + $request = new Request(); + $response = $controller->putOnHold($this->entityWorkflow, $request); $this->assertInstanceOf(Response::class, $response); - $this->assertEquals(302, $response->getStatusCode());*/ + $this->assertEquals(302, $response->getStatusCode()); } } From 745a29f74284623e9860733274cb4cf78753b56a Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Fri, 30 Aug 2024 12:59:08 +0200 Subject: [PATCH 16/21] Adjust logic for removing the hold on a workflow only by user who owns the hold and when a transition is applied on the workflow --- .../Controller/WorkflowController.php | 8 ++++-- .../Controller/WorkflowOnHoldController.php | 28 +++++++++---------- .../EntityWorkflowStepHoldRepository.php | 19 +++++++++++++ .../Resources/views/Workflow/index.html.twig | 4 +-- .../WorkflowOnHoldControllerTest.php | 15 +++++----- 5 files changed, 48 insertions(+), 26 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php index 71ff897e0..6c9ee2f8c 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php @@ -298,7 +298,7 @@ class WorkflowController extends AbstractController $workflow = $this->registry->get($entityWorkflow, $entityWorkflow->getWorkflowName()); $errors = []; $signatures = $entityWorkflow->getCurrentStep()->getSignatures(); - $holdOnStepByUser = $this->entityWorkflowStepHoldRepository->findOneByStepAndUser($entityWorkflow->getCurrentStep(), $this->security->getUser()); + $onHoldStep = $this->entityWorkflowStepHoldRepository->findByWorkflow($entityWorkflow); if (\count($workflow->getEnabledTransitions($entityWorkflow)) > 0) { // possible transition @@ -343,6 +343,10 @@ class WorkflowController extends AbstractController $this->entityManager->flush(); + if ($onHoldStep) { + $this->entityManager->remove($onHoldStep); + } + return $this->redirectToRoute('chill_main_workflow_show', ['id' => $entityWorkflow->getId()]); } @@ -361,7 +365,7 @@ class WorkflowController extends AbstractController 'entity_workflow' => $entityWorkflow, 'transition_form_errors' => $errors, 'signatures' => $signatures, - 'holdOnStepByUser' => $holdOnStepByUser, + 'onHoldStep' => $onHoldStep, ] ); } diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php index a7831159c..36c419e79 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php @@ -4,6 +4,7 @@ namespace Chill\MainBundle\Controller; use Chill\MainBundle\Entity\Workflow\EntityWorkflow; use Chill\MainBundle\Entity\Workflow\EntityWorkflowStepHold; +use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use Chill\MainBundle\Repository\Workflow\EntityWorkflowStepHoldRepository; use Chill\MainBundle\Security\ChillSecurity; use Doctrine\ORM\EntityManagerInterface; @@ -20,32 +21,23 @@ class WorkflowOnHoldController extends AbstractController private readonly EntityManagerInterface $entityManager, private readonly Security $security, private readonly Registry $registry, - private readonly EntityWorkflowStepHoldRepository $entityWorkflowStepHoldRepository + private readonly EntityWorkflowStepHoldRepository $entityWorkflowStepHoldRepository, + 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(); $workflow = $this->registry->get($entityWorkflow, $entityWorkflow->getWorkflowName()); - $enabledTransitions = $workflow->getEnabledTransitions($entityWorkflow); - if (\count($enabledTransitions) === 0) { - throw $this->createAccessDeniedException('No transitions are available for the current workflow state.'); - } - $isTransitionAllowed = false; - foreach ($enabledTransitions as $transition) { - if ($workflow->can($entityWorkflow, $transition->getName())) { - $isTransitionAllowed = true; - break; - } - } - - if (!$isTransitionAllowed) { - throw $this->createAccessDeniedException('You are not allowed to apply any transitions to this workflow, therefore you cannot put it on hold.'); + if (!count($enabledTransitions) > 0) { + throw $this->createAccessDeniedException('You are not allowed to apply any transitions to this workflow, therefore you cannot toggle the hold status.'); } $stepHold = new EntityWorkflowStepHold($currentStep, $currentUser); @@ -61,10 +53,16 @@ class WorkflowOnHoldController extends AbstractController { $hold = $this->entityWorkflowStepHoldRepository->findById($holdId); $entityWorkflow = $hold->getStep()->getEntityWorkflow(); + $currentUser = $this->security->getUser(); + + if ($hold->getByUser() !== $currentUser) { + throw $this->createAccessDeniedException('You are not allowed to remove the hold status.'); + } $this->entityManager->remove($hold); $this->entityManager->flush(); return $this->redirectToRoute('chill_main_workflow_show', ['id' => $entityWorkflow->getId()]); } + } diff --git a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php index 9e1316e05..4b6cc7d1c 100644 --- a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php +++ b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php @@ -12,12 +12,14 @@ 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; use Doctrine\ORM\NonUniqueResultException; use Doctrine\ORM\NoResultException; use Doctrine\Persistence\ManagerRegistry; +use Symfony\Component\Workflow\Workflow; /** * @template-extends ServiceEntityRepository @@ -58,6 +60,23 @@ 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. * diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig index 191499b0a..20f617c0e 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig @@ -68,9 +68,9 @@
{% include '@ChillMain/Workflow/_history.html.twig' %}
    - {% if holdOnStepByUser|length > 0 %} + {% if onHoldStep is not null %}
  • - + {{ 'workflow.Remove hold'|trans }}
  • diff --git a/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php b/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php index 785d5d33c..5ca829029 100644 --- a/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php @@ -30,6 +30,7 @@ class WorkflowOnHoldControllerTest extends KernelTestCase private $workflow; private $currentStep; private $currentUser; + private $controller; protected function setUp(): void { @@ -53,6 +54,13 @@ class WorkflowOnHoldControllerTest extends KernelTestCase $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 + ); } public function testPutOnHoldPersistence(): void @@ -76,13 +84,6 @@ class WorkflowOnHoldControllerTest extends KernelTestCase $this->entityManager->expects($this->once()) ->method('flush'); - $controller = new WorkflowOnHoldController( - $this->entityManager, - $this->security, - $this->registry, - $this->entityWorkflowStepHoldRepository - ); - $request = new Request(); $controller->putOnHold($this->entityWorkflow, $request); } From 8c4f342ca1c69148f1e2e4cf6a5290b2c3c3bdfd Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Fri, 30 Aug 2024 13:02:39 +0200 Subject: [PATCH 17/21] Correct instantiation of controller within test --- .../Controller/WorkflowOnHoldControllerTest.php | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php b/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php index 5ca829029..8c06e9f90 100644 --- a/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php @@ -6,6 +6,7 @@ use Chill\MainBundle\Controller\WorkflowOnHoldController; 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\Security\ChillSecurity; use Doctrine\ORM\EntityManagerInterface; @@ -41,6 +42,7 @@ class WorkflowOnHoldControllerTest extends KernelTestCase $this->security = $this->createMock(Security::class); $this->registry = $this->createMock(Registry::class); $this->entityWorkflowStepHoldRepository = $this->createMock(EntityWorkflowStepHoldRepository::class); + $this->entityWorkflowRepository = $this->createMock(EntityWorkflowRepository::class); $this->entityWorkflow = $this->createMock(EntityWorkflow::class); $this->workflow = $this->createMock(Workflow::class); @@ -59,7 +61,8 @@ class WorkflowOnHoldControllerTest extends KernelTestCase $this->entityManager, $this->security, $this->registry, - $this->entityWorkflowStepHoldRepository + $this->entityWorkflowStepHoldRepository, + $this->entityWorkflowRepository ); } @@ -85,20 +88,13 @@ class WorkflowOnHoldControllerTest extends KernelTestCase ->method('flush'); $request = new Request(); - $controller->putOnHold($this->entityWorkflow, $request); + $this->controller->putOnHold($this->entityWorkflow, $request); } public function testPutOnHoldSmokeTest(): void { - $controller = new WorkflowOnHoldController( - $this->entityManager, - $this->security, - $this->registry, - $this->entityWorkflowStepHoldRepository - ); - $request = new Request(); - $response = $controller->putOnHold($this->entityWorkflow, $request); + $response = $this->controller->putOnHold($this->entityWorkflow, $request); $this->assertInstanceOf(Response::class, $response); $this->assertEquals(302, $response->getStatusCode()); From e29e1db6ed337cdf3856db8d7fcaa24c1351a830 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Thu, 5 Sep 2024 17:38:18 +0200 Subject: [PATCH 18/21] Take into account 'destUserByAccessKey' in the list of workflows associated to a user Refactor the query to include checks for user membership via both 'destUser' and 'destUserByAccessKey'. This ensures that workflows correctly account for user access by multiple criteria. --- .../Repository/Workflow/EntityWorkflowRepository.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php index 81cdcd551..f5696d2b9 100644 --- a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php +++ b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php @@ -230,7 +230,10 @@ class EntityWorkflowRepository implements ObjectRepository $qb->where( $qb->expr()->andX( - $qb->expr()->isMemberOf(':user', 'step.destUser'), + $qb->expr()->orX( + $qb->expr()->isMemberOf(':user', 'step.destUser'), + $qb->expr()->isMemberOf(':user', 'step.destUserByAccessKey'), + ), $qb->expr()->isNull('step.transitionAfter'), $qb->expr()->eq('step.isFinal', "'FALSE'") ) From e8f09b507f92c70b72a8d3f68b2cb55bb45abec2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Thu, 5 Sep 2024 18:00:37 +0200 Subject: [PATCH 19/21] Fix cs after php-cs-fixer upgrade, fix phpstan and rector errors --- .../Controller/WorkflowController.php | 2 +- .../Controller/WorkflowOnHoldController.php | 21 +++++++++++++++---- .../Workflow/EntityWorkflowStepHold.php | 20 ++++++------------ .../EntityWorkflowStepHoldRepository.php | 4 +--- .../WorkflowOnHoldControllerTest.php | 18 ++++++++++++---- 5 files changed, 39 insertions(+), 26 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php index 6c9ee2f8c..08776fcba 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php @@ -343,7 +343,7 @@ class WorkflowController extends AbstractController $this->entityManager->flush(); - if ($onHoldStep) { + if (null !== $onHoldStep) { $this->entityManager->remove($onHoldStep); } diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php index 36c419e79..75c9fd380 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php @@ -1,16 +1,26 @@ getCurrentStep(); $currentUser = $this->security->getUser(); + if (!$currentUser instanceof User) { + throw new AccessDeniedHttpException('only user can put a workflow on hold'); + } + $workflow = $this->registry->get($entityWorkflow, $entityWorkflow->getWorkflowName()); $enabledTransitions = $workflow->getEnabledTransitions($entityWorkflow); - if (!count($enabledTransitions) > 0) { + 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.'); } @@ -64,5 +78,4 @@ class WorkflowOnHoldController extends AbstractController return $this->redirectToRoute('chill_main_workflow_show', ['id' => $entityWorkflow->getId()]); } - } diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php index 47ae0f954..3f4aeb669 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepHold.php @@ -26,21 +26,13 @@ class EntityWorkflowStepHold implements TrackCreationInterface #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column(type: \Doctrine\DBAL\Types\Types::INTEGER)] - private ?int $id; + private ?int $id = null; - #[ORM\ManyToOne(targetEntity: EntityWorkflowStep::class)] - #[ORM\JoinColumn(nullable: false)] - private EntityWorkflowStep $step; - - #[ORM\ManyToOne(targetEntity: User::class)] - #[ORM\JoinColumn(nullable: false)] - private User $byUser; - - public function __construct(EntityWorkflowStep $step, User $byUser) - { - $this->step = $step; - $this->byUser = $byUser; - } + public function __construct(#[ORM\ManyToOne(targetEntity: EntityWorkflowStep::class)] + #[ORM\JoinColumn(nullable: false)] + private EntityWorkflowStep $step, #[ORM\ManyToOne(targetEntity: User::class)] + #[ORM\JoinColumn(nullable: false)] + private User $byUser) {} public function getId(): ?int { diff --git a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php index 4b6cc7d1c..7ef877181 100644 --- a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php +++ b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php @@ -19,12 +19,10 @@ use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; use Doctrine\ORM\NonUniqueResultException; use Doctrine\ORM\NoResultException; use Doctrine\Persistence\ManagerRegistry; -use Symfony\Component\Workflow\Workflow; /** * @template-extends ServiceEntityRepository */ - class EntityWorkflowStepHoldRepository extends ServiceEntityRepository { public function __construct(ManagerRegistry $registry) @@ -92,7 +90,7 @@ class EntityWorkflowStepHoldRepository extends ServiceEntityRepository ->setParameter('user', $user) ->getQuery() ->getSingleResult(); - } catch (NoResultException $e) { + } catch (NoResultException) { return null; } } diff --git a/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php b/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php index 8c06e9f90..f4788c2ec 100644 --- a/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Controller/WorkflowOnHoldControllerTest.php @@ -1,5 +1,14 @@ Date: Thu, 5 Sep 2024 20:46:29 +0200 Subject: [PATCH 20/21] 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. --- .../Controller/WorkflowController.php | 8 ------ .../Controller/WorkflowOnHoldController.php | 27 +++++++++++++------ .../Entity/Workflow/EntityWorkflow.php | 7 +++-- .../Entity/Workflow/EntityWorkflowStep.php | 11 ++++++++ .../EntityWorkflowStepHoldRepository.php | 18 ------------- .../Resources/views/Workflow/index.html.twig | 4 +-- 6 files changed, 37 insertions(+), 38 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php index 08776fcba..ab3d67307 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php +++ b/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, ] ); } diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php index 75c9fd380..6e4af7e22 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php +++ b/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')] - public function removeOnHold(int $holdId, Request $request): Response + #[Route(path: '/{_locale}/main/workflow/{id}/remove_hold', name: 'chill_main_workflow_remove_hold')] + public function removeOnHold(EntityWorkflowStep $entityWorkflowStep, Request $request): Response { - $hold = $this->entityWorkflowStepHoldRepository->findById($holdId); - $entityWorkflow = $hold->getStep()->getEntityWorkflow(); - $currentUser = $this->security->getUser(); + $user = $this->security->getUser(); - if ($hold->getByUser() !== $currentUser) { - throw $this->createAccessDeniedException('You are not allowed to remove the hold status.'); + if (!$user instanceof User) { + 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()]); } } diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php index 14b39a065..1f32a1985 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php +++ b/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); diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php index 1cdb157b1..90aa632f8 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStep.php +++ b/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) { diff --git a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php index 7ef877181..a925246e4 100644 --- a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowStepHoldRepository.php +++ b/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. * diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig index 20f617c0e..c5daff201 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/index.html.twig @@ -68,9 +68,9 @@
    {% include '@ChillMain/Workflow/_history.html.twig' %}
      - {% if onHoldStep is not null %} + {% if entity_workflow.isOnHoldByUser(app.user) %}
    • - + {{ 'workflow.Remove hold'|trans }}
    • 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 21/21] 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()); } }