From 3e6d764b9b3d5ef4cbe97d4bbd7fb7dff2799f46 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 7 Aug 2024 12:44:34 +0200 Subject: [PATCH 01/60] Replace sign button with signed statement if person/user has signed --- .../Controller/WorkflowController.php | 10 ++++++++-- .../views/Workflow/_signature.html.twig | 16 ++++++++++------ .../ChillMainBundle/translations/messages.fr.yml | 1 + 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php index 277a18142..9f2196982 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php @@ -14,11 +14,11 @@ 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\EntityWorkflowStepSignature; use Chill\MainBundle\Form\WorkflowSignatureMetadataType; 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\Security\Authorization\EntityWorkflowVoter; use Chill\MainBundle\Security\ChillSecurity; use Chill\MainBundle\Workflow\EntityWorkflowManager; @@ -32,6 +32,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\Routing\Annotation\Route; use Symfony\Component\Validator\Validator\ValidatorInterface; use Symfony\Component\Workflow\Registry; @@ -51,6 +52,7 @@ class WorkflowController extends AbstractController private readonly ChillSecurity $security, private readonly \Doctrine\Persistence\ManagerRegistry $managerRegistry, private readonly ClockInterface $clock, + private readonly EntityWorkflowStepSignatureRepository $entityWorkflowStepSignatureRepository, ) {} #[Route(path: '/{_locale}/main/workflow/create', name: 'chill_main_workflow_create')] @@ -374,7 +376,11 @@ class WorkflowController extends AbstractController #[Route(path: '/{_locale}/main/workflow/signature/{signature_id}/metadata', name: 'chill_main_workflow_signature_metadata')] public function addSignatureMetadata(int $signature_id, Request $request): Response { - $signature = $this->entityManager->getRepository(EntityWorkflowStepSignature::class)->find($signature_id); + $signature = $this->entityWorkflowStepSignatureRepository->find($signature_id); + + if (null === $signature) { + throw new NotFoundHttpException('signature not found'); + } if ($signature->getSigner() instanceof User) { return $this->redirectToRoute('chill_main_workflow_signature_add', ['id' => $signature_id]); diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig index f613b69dd..ddccc6a4c 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig @@ -6,12 +6,16 @@
{{ s.signer|chill_entity_render_box }}
{% endfor %} diff --git a/src/Bundle/ChillMainBundle/translations/messages.fr.yml b/src/Bundle/ChillMainBundle/translations/messages.fr.yml index 9a90b7fa1..23a77c710 100644 --- a/src/Bundle/ChillMainBundle/translations/messages.fr.yml +++ b/src/Bundle/ChillMainBundle/translations/messages.fr.yml @@ -531,6 +531,7 @@ workflow: signature_zone: title: Appliquer les signatures électroniques button_sign: Signer + has_signed_statement: 'À signé le %datetime%' metadata: sign_by: 'Signature pour %name%' docType: Type de document From ee6edba2067982a9968709f80a2378ab8e651d29 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 7 Aug 2024 14:18:16 +0200 Subject: [PATCH 02/60] Create isSigned method in EntityWorkflowStepSignature.php Refactorization and ease of use in template and workflow controller --- .../Entity/Workflow/EntityWorkflowStepSignature.php | 5 +++++ .../Resources/views/Workflow/_signature.html.twig | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php index 46c8bb04d..527ede0ef 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php @@ -135,4 +135,9 @@ class EntityWorkflowStepSignature implements TrackCreationInterface, TrackUpdate return $this; } + + public function isSigned(): bool + { + return EntityWorkflowSignatureStateEnum::SIGNED == $this->getState(); + } } diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig index ddccc6a4c..e439f2268 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig @@ -6,7 +6,7 @@
{{ s.signer|chill_entity_render_box }}
    - {% if s.state.value == 'signed' %} + {% if s.isSigned %}

    {{ 'workflow.signature_zone.has_signed_statement'|trans( { '%datetime%' : s.stateDate|date('j M Y à H:i:s') } ) }}

    {% else %}
  • From 0c797c29970d68dd0ea7b434e7eb4e98827e7696 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 7 Aug 2024 14:18:34 +0200 Subject: [PATCH 03/60] Redirect to workflow show page if document already signed Verify the state of the signature. If isSigned is true, redirection to workflow show page. --- src/Bundle/ChillMainBundle/Controller/WorkflowController.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php index 9f2196982..b9c728eeb 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php @@ -382,6 +382,10 @@ class WorkflowController extends AbstractController throw new NotFoundHttpException('signature not found'); } + if ($signature->isSigned()) { + return $this->redirectToRoute('chill_main_workflow_show', ['id' => $signature->getStep()->getEntityWorkflow()->getId()]); + } + if ($signature->getSigner() instanceof User) { return $this->redirectToRoute('chill_main_workflow_signature_add', ['id' => $signature_id]); } From cc8214d52cc9c7de5d5b020e46bc82ec203e5048 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 28 Aug 2024 16:13:26 +0200 Subject: [PATCH 04/60] Add flash message in case signature was already applied --- src/Bundle/ChillMainBundle/Controller/WorkflowController.php | 4 ++++ src/Bundle/ChillMainBundle/translations/messages.fr.yml | 1 + 2 files changed, 5 insertions(+) diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php index b9c728eeb..5d78103b8 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php @@ -383,6 +383,10 @@ class WorkflowController extends AbstractController } if ($signature->isSigned()) { + $this->addFlash( + 'notice', + $this->translator->trans('workflow.signature_zone.already_signed_alert') + ); return $this->redirectToRoute('chill_main_workflow_show', ['id' => $signature->getStep()->getEntityWorkflow()->getId()]); } diff --git a/src/Bundle/ChillMainBundle/translations/messages.fr.yml b/src/Bundle/ChillMainBundle/translations/messages.fr.yml index 23a77c710..e442f38aa 100644 --- a/src/Bundle/ChillMainBundle/translations/messages.fr.yml +++ b/src/Bundle/ChillMainBundle/translations/messages.fr.yml @@ -542,6 +542,7 @@ workflow: user signature: Selectionner utilisateur pour signer persons: Usagers user: Utilisateur + already_signed_alert: La signature a déjà été appliquée Subscribe final: Recevoir une notification à l'étape finale From ce781a5b587859b333d11b671e3bc48ca93b3a68 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 28 Aug 2024 16:27:04 +0200 Subject: [PATCH 05/60] Translate datetime object with icu: split date and time Translate datetime within icu format --- src/Bundle/ChillMainBundle/Controller/WorkflowController.php | 1 + .../Resources/views/Workflow/_signature.html.twig | 2 +- .../ChillMainBundle/translations/messages+intl-icu.fr.yaml | 3 +++ src/Bundle/ChillMainBundle/translations/messages.fr.yml | 1 - 4 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php index 5d78103b8..eb72c2624 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowController.php +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowController.php @@ -387,6 +387,7 @@ class WorkflowController extends AbstractController 'notice', $this->translator->trans('workflow.signature_zone.already_signed_alert') ); + return $this->redirectToRoute('chill_main_workflow_show', ['id' => $signature->getStep()->getEntityWorkflow()->getId()]); } diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig index e439f2268..a2a4129f1 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig @@ -7,7 +7,7 @@
      {% if s.isSigned %} -

      {{ 'workflow.signature_zone.has_signed_statement'|trans( { '%datetime%' : s.stateDate|date('j M Y à H:i:s') } ) }}

      +

      {{ 'workflow.signature_zone.has_signed_statement'|trans({ 'datetime' : s.stateDate }) }}

      {% else %}
    • {{ 'workflow.signature_zone.button_sign'|trans }} diff --git a/src/Bundle/ChillMainBundle/translations/messages+intl-icu.fr.yaml b/src/Bundle/ChillMainBundle/translations/messages+intl-icu.fr.yaml index 96b2edd98..a0753f7a6 100644 --- a/src/Bundle/ChillMainBundle/translations/messages+intl-icu.fr.yaml +++ b/src/Bundle/ChillMainBundle/translations/messages+intl-icu.fr.yaml @@ -45,6 +45,9 @@ workflow: few {# workflows} other {# workflows} } + signature_zone: + has_signed_statement: 'A signé le {datetime, date, short} à {datetime, time, short}' + duration: minute: >- diff --git a/src/Bundle/ChillMainBundle/translations/messages.fr.yml b/src/Bundle/ChillMainBundle/translations/messages.fr.yml index e442f38aa..f6f30bcd0 100644 --- a/src/Bundle/ChillMainBundle/translations/messages.fr.yml +++ b/src/Bundle/ChillMainBundle/translations/messages.fr.yml @@ -531,7 +531,6 @@ workflow: signature_zone: title: Appliquer les signatures électroniques button_sign: Signer - has_signed_statement: 'À signé le %datetime%' metadata: sign_by: 'Signature pour %name%' docType: Type de document From 71d3aa3969936cb6d98e7702d439d4ada173a238 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 4 Sep 2024 17:24:11 +0200 Subject: [PATCH 06/60] Refactor signature rendering logic Reorganized the signature rendering loop for better readability. Moved the row alignment inside the loop and added text alignment for signed statements. Simplified the conditional checks within the loop to enhance code maintainability. --- .../views/Workflow/_signature.html.twig | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig index a2a4129f1..2452411d5 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig @@ -1,24 +1,24 @@

      {{ 'workflow.signature_zone.title'|trans }}

      -
      - {% for s in signatures %} -
      {{ s.signer|chill_entity_render_box }}
      -
      -
        - {% if s.isSigned %} -

        {{ 'workflow.signature_zone.has_signed_statement'|trans({ 'datetime' : s.stateDate }) }}

        - {% else %} -
      • - {{ 'workflow.signature_zone.button_sign'|trans }} - {% if s.state is same as('signed') %} -

        {{ s.stateDate }}

        - {% endif %} -
        • - {% endif %} -
      -
      - {% endfor %} -
      + {% for s in signatures %} +
      +
      {{ s.signer|chill_entity_render_box }}
      +
      + {% if s.isSigned %} +

      {{ 'workflow.signature_zone.has_signed_statement'|trans({ 'datetime' : s.stateDate }) }}

      + {% else %} + + {% endif %} +
      +
      + {% endfor %}
      From 20f2bc6c35e3b2b3d5f46d2dd50ad795ab341d12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 4 Sep 2024 17:26:46 +0200 Subject: [PATCH 07/60] Enhance signature display with detailed person information Updated the signature view template to include person details using the '_insert_vue_onthefly.html.twig' template. This change adds more contextual information about the signer, such as their name and status, improving the user experience. --- .../Resources/views/Workflow/_signature.html.twig | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig index 2452411d5..a938f9e05 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig @@ -3,7 +3,14 @@
      {% for s in signatures %}
      -
      {{ s.signer|chill_entity_render_box }}
      +
      + {% include '@ChillMain/OnTheFly/_insert_vue_onthefly.html.twig' with { + action: 'show', displayBadge: true, + targetEntity: { name: 'person', id: s.signer.id }, + buttonText: s.signer|chill_entity_render_string, + isDead: s.signer.deathDate is not null + } %} +
      {% if s.isSigned %}

      {{ 'workflow.signature_zone.has_signed_statement'|trans({ 'datetime' : s.stateDate }) }}

      From d0031e82e8eb5e8d6f9610361204a315a9fb17a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 4 Sep 2024 17:55:01 +0200 Subject: [PATCH 08/60] Add hover effect and slim class to dev assets and apply in signature list Introduced a new SCSS file to handle hover effects on rows and added the ability to remove bottom margins with a "slim" class. Updated various twig templates to utilize these new styles for better visual feedback and alignment. --- .../Resources/public/chill/chillmain.scss | 2 + .../Resources/public/chill/scss/hover.scss | 11 +++ .../public/chill/scss/record_actions.scss | 4 + .../Resources/views/Dev/dev.assets.html.twig | 89 +++++++++++++++++++ .../views/Workflow/_signature.html.twig | 6 +- 5 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 src/Bundle/ChillMainBundle/Resources/public/chill/scss/hover.scss diff --git a/src/Bundle/ChillMainBundle/Resources/public/chill/chillmain.scss b/src/Bundle/ChillMainBundle/Resources/public/chill/chillmain.scss index 2523ee202..eabc2adc4 100644 --- a/src/Bundle/ChillMainBundle/Resources/public/chill/chillmain.scss +++ b/src/Bundle/ChillMainBundle/Resources/public/chill/chillmain.scss @@ -31,6 +31,8 @@ // Specific templates @import './scss/notification'; +@import './scss/hover.scss'; + /* * BASE LAYOUT POSITION */ diff --git a/src/Bundle/ChillMainBundle/Resources/public/chill/scss/hover.scss b/src/Bundle/ChillMainBundle/Resources/public/chill/scss/hover.scss new file mode 100644 index 000000000..95fe12efc --- /dev/null +++ b/src/Bundle/ChillMainBundle/Resources/public/chill/scss/hover.scss @@ -0,0 +1,11 @@ + + +.row.row-hover { + padding: 0.3rem; + + &:hover { + background-color: $gray-100; + border-top: 1px solid $gray-400; + border-bottom: 1px solid $gray-400; + } +} diff --git a/src/Bundle/ChillMainBundle/Resources/public/chill/scss/record_actions.scss b/src/Bundle/ChillMainBundle/Resources/public/chill/scss/record_actions.scss index 5158a826e..d37765e6b 100644 --- a/src/Bundle/ChillMainBundle/Resources/public/chill/scss/record_actions.scss +++ b/src/Bundle/ChillMainBundle/Resources/public/chill/scss/record_actions.scss @@ -17,6 +17,10 @@ ul.record_actions { display: inline-block; } + &.slim { + margin-bottom: 0; + } + &.column { flex-direction: column; } diff --git a/src/Bundle/ChillMainBundle/Resources/views/Dev/dev.assets.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Dev/dev.assets.html.twig index 6a7c4edf0..5aafb6635 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Dev/dev.assets.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Dev/dev.assets.html.twig @@ -300,7 +300,96 @@
    +

    slim

    + +

    Ajouter slim enlève la marge inférieure. Permet un meilleur alignement horizontal dans une row

    + +
    +
    +
    + Some text, ul_record_actions sans slim +
    +
    +
      +
      • +
    +
    +
    +
    +
    + Some text, ul_record_actions avec slim +
    +
    +
      +
      • +
    +
    +
    +
    + + + <a class="btn btn-submit">Text</a> Toutes les classes btn-* de bootstrap sont fonctionnelles + +

    Hover

    + +

    Ajouter .row-hover sur une class .row provoque un changement de background au survol

    + +
    +
    +
    + Fatoumata Binta DIALLO (33 ans) +
    +
    + A signé le 04/09/2024 à 13:55 +
    +
    +
    +
    + Abdoulaye DIALLO (9 ans) +
    +
    + +
    +
    +
    +
    + Mohamed DIALLO (44 ans) +
    +
    + A signé le 04/09/2024 à 13:57 +
    +
    +
    +
    + Fatou DIALLO (37 ans) +
    +
    + +
    +
    +
    +
    + Fanta DIALLO (7 ans) +
    +
    + +
    +
    +
+ {% endblock %} diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig index a938f9e05..68b6f4274 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig @@ -2,7 +2,7 @@
{% for s in signatures %} -
+
{% include '@ChillMain/OnTheFly/_insert_vue_onthefly.html.twig' with { action: 'show', displayBadge: true, @@ -13,9 +13,9 @@
{% if s.isSigned %} -

{{ 'workflow.signature_zone.has_signed_statement'|trans({ 'datetime' : s.stateDate }) }}

+ {{ 'workflow.signature_zone.has_signed_statement'|trans({ 'datetime' : s.stateDate }) }} {% else %} -
    +
    • {{ 'workflow.signature_zone.button_sign'|trans }} {% if s.state is same as('signed') %} From 8ea87053f04ab9c0ff58ead24558e0a8005d1d9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Thu, 5 Sep 2024 16:47:45 +0200 Subject: [PATCH 09/60] fix some tests --- .../Tests/Service/StoredObjectManagerTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectManagerTest.php b/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectManagerTest.php index e09a2dc1e..39412b9f7 100644 --- a/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectManagerTest.php +++ b/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectManagerTest.php @@ -220,7 +220,8 @@ final class StoredObjectManagerTest extends TestCase ->willReturnCallback(fn (string $method, string $objectName) => new SignedUrl( $method, 'https://example.com/'.$objectName, - new \DateTimeImmutable('1 hours') + new \DateTimeImmutable('1 hours'), + $objectName )); $storedObjectManager = new StoredObjectManager($httpClient, $tempUrlGenerator); @@ -306,7 +307,8 @@ final class StoredObjectManagerTest extends TestCase ->willReturnCallback(fn (string $method, string $objectName) => new SignedUrl( $method, 'https://example.com/'.$objectName, - new \DateTimeImmutable('1 hours') + new \DateTimeImmutable('1 hours'), + $objectName )); $manager = new StoredObjectManager($client, $tempUrlGenerator); From 9475a708c39bc4f955947bc27b559c080c5bd2e5 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 7 Aug 2024 16:47:29 +0200 Subject: [PATCH 10/60] 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 11/60] 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 12/60] 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 13/60] 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 14/60] 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 15/60] 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 16/60] 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 17/60] 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 18/60] 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 19/60] 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 20/60] 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 21/60] 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 22/60] 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 23/60] 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 24/60] 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 25/60] 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 26/60] 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 27/60] 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 28/60] 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 29/60] 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 30/60] 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()); } } From 4d73f9b81af03b9cfb30188bfd6a6a7a26ca3e8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Fri, 6 Sep 2024 14:07:51 +0200 Subject: [PATCH 31/60] Rename Convert to ConvertController for clarity Renamed Convert class and relevant references to ConvertController to improve clarity and maintain consistency. Updated corresponding test files and route configurations to reflect the new name. --- .../{Convert.php => ConvertController.php} | 2 +- .../src/Resources/config/routes/routes.php | 2 +- .../{ConvertTest.php => ConvertControllerTest.php} | 14 ++++++++------ 3 files changed, 10 insertions(+), 8 deletions(-) rename src/Bundle/ChillWopiBundle/src/Controller/{Convert.php => ConvertController.php} (99%) rename src/Bundle/ChillWopiBundle/tests/Controller/{ConvertTest.php => ConvertControllerTest.php} (86%) diff --git a/src/Bundle/ChillWopiBundle/src/Controller/Convert.php b/src/Bundle/ChillWopiBundle/src/Controller/ConvertController.php similarity index 99% rename from src/Bundle/ChillWopiBundle/src/Controller/Convert.php rename to src/Bundle/ChillWopiBundle/src/Controller/ConvertController.php index 16be18803..73207e13a 100644 --- a/src/Bundle/ChillWopiBundle/src/Controller/Convert.php +++ b/src/Bundle/ChillWopiBundle/src/Controller/ConvertController.php @@ -31,7 +31,7 @@ use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface; use Symfony\Contracts\HttpClient\HttpClientInterface; use Symfony\Contracts\HttpClient\ResponseInterface; -class Convert +class ConvertController { private const LOG_PREFIX = '[convert] '; diff --git a/src/Bundle/ChillWopiBundle/src/Resources/config/routes/routes.php b/src/Bundle/ChillWopiBundle/src/Resources/config/routes/routes.php index 2272c9efd..62837a3ce 100644 --- a/src/Bundle/ChillWopiBundle/src/Resources/config/routes/routes.php +++ b/src/Bundle/ChillWopiBundle/src/Resources/config/routes/routes.php @@ -19,5 +19,5 @@ return static function (RoutingConfigurator $routes) { $routes ->add('chill_wopi_object_convert', '/convert/{uuid}') - ->controller(Chill\WopiBundle\Controller\Convert::class); + ->controller(Chill\WopiBundle\Controller\ConvertController::class); }; diff --git a/src/Bundle/ChillWopiBundle/tests/Controller/ConvertTest.php b/src/Bundle/ChillWopiBundle/tests/Controller/ConvertControllerTest.php similarity index 86% rename from src/Bundle/ChillWopiBundle/tests/Controller/ConvertTest.php rename to src/Bundle/ChillWopiBundle/tests/Controller/ConvertControllerTest.php index 2bc3f4769..0a928bcd9 100644 --- a/src/Bundle/ChillWopiBundle/tests/Controller/ConvertTest.php +++ b/src/Bundle/ChillWopiBundle/tests/Controller/ConvertControllerTest.php @@ -14,7 +14,7 @@ namespace Chill\WopiBundle\Tests\Controller; use Chill\DocStoreBundle\Entity\StoredObject; use Chill\DocStoreBundle\Service\StoredObjectManagerInterface; use Chill\MainBundle\Entity\User; -use Chill\WopiBundle\Controller\Convert; +use Chill\WopiBundle\Controller\ConvertController; use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; use Psr\Log\NullLogger; @@ -30,13 +30,14 @@ use Symfony\Component\Security\Core\Security; * * @coversNothing */ -final class ConvertTest extends TestCase +final class ConvertControllerTest extends TestCase { use ProphecyTrait; public function testConversionFailed(): void { - $storedObject = (new StoredObject())->setType('application/vnd.oasis.opendocument.text'); + $storedObject = new StoredObject(); + $storedObject->registerVersion(type: 'application/vnd.oasis.opendocument.text'); $httpClient = new MockHttpClient([ new MockResponse('not authorized', ['http_code' => 401]), @@ -50,7 +51,7 @@ final class ConvertTest extends TestCase $parameterBag = new ParameterBag(['wopi' => ['server' => 'http://collabora:9980']]); - $convert = new Convert( + $convert = new ConvertController( $httpClient, $this->makeRequestStack(), $security->reveal(), @@ -66,7 +67,8 @@ final class ConvertTest extends TestCase public function testEverythingWentFine(): void { - $storedObject = (new StoredObject())->setType('application/vnd.oasis.opendocument.text'); + $storedObject = new StoredObject(); + $storedObject->registerVersion(type: 'application/vnd.oasis.opendocument.text'); $httpClient = new MockHttpClient([ new MockResponse('1234', ['http_code' => 200]), @@ -80,7 +82,7 @@ final class ConvertTest extends TestCase $parameterBag = new ParameterBag(['wopi' => ['server' => 'http://collabora:9980']]); - $convert = new Convert( + $convert = new ConvertController( $httpClient, $this->makeRequestStack(), $security->reveal(), From bf056046abea186d67169c0de40c53af623dbd13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Fri, 6 Sep 2024 14:40:25 +0200 Subject: [PATCH 32/60] Fix cs --- .../ChillMainBundle/Controller/WorkflowOnHoldController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php b/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php index 08f62e53a..077b8effa 100644 --- a/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php +++ b/src/Bundle/ChillMainBundle/Controller/WorkflowOnHoldController.php @@ -78,7 +78,7 @@ class WorkflowOnHoldController throw new AccessDeniedHttpException('You are not allowed to remove workflow on hold'); } - $hold = $entityWorkflowStep->getHoldsOnStep()->findFirst(fn(int $index, EntityWorkflowStepHold $entityWorkflowStepHold) => $user === $entityWorkflowStepHold->getByUser()); + $hold = $entityWorkflowStep->getHoldsOnStep()->findFirst(fn (int $index, EntityWorkflowStepHold $entityWorkflowStepHold) => $user === $entityWorkflowStepHold->getByUser()); if (null === $hold) { // this should not happens... From 860ae5cedfda6aa67511555d06362b0f502e2afb Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Thu, 8 Aug 2024 17:00:27 +0200 Subject: [PATCH 33/60] Create CancelStaleWorkflow message and handler --- .../Service/Workflow/CancelStaleWorkflow.php | 15 ++++++++++ .../Workflow/CancelStaleWorkflowHandler.php | 30 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php create mode 100644 src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php new file mode 100644 index 000000000..d13ca039e --- /dev/null +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php @@ -0,0 +1,15 @@ +workflowIds; + } +} diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php new file mode 100644 index 000000000..aff22d1e3 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php @@ -0,0 +1,30 @@ +getWorkflowIds(); + + foreach ($workflowIds as $workflowId) { + $workflow = $this->workflowRepository->find($workflowId); + + $steps = $workflow->getSteps(); + + + } + } + +} From 34edb02cd06ea8bfcf662b14123682017ad9c625 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Mon, 12 Aug 2024 11:44:31 +0200 Subject: [PATCH 34/60] Create message and handler for canceling stale workflows --- .../Service/Workflow/CancelStaleWorkflow.php | 6 +-- .../Workflow/CancelStaleWorkflowHandler.php | 43 +++++++++++++++---- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php index d13ca039e..078402855 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php @@ -4,12 +4,12 @@ namespace Chill\MainBundle\Service\Workflow; class CancelStaleWorkflow { - public function __construct(public array $workflowIds) + public function __construct(public int $workflowId) { } - public function getWorkflowIds(): array + public function getWorkflowId(): int { - return $this->workflowIds; + return $this->workflowId; } } diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php index aff22d1e3..e35464f1d 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php @@ -5,26 +5,53 @@ namespace Chill\MainBundle\Service\Workflow; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; -use Symfony\Component\Messenger\Handler\MessageHandlerInterface; +use Symfony\Component\Messenger\Attribute\AsMessageHandler; +use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException; +use Symfony\Component\Workflow\Registry; -class CancelStaleWorkflowHandler implements MessageHandlerInterface +#[AsMessageHandler] +class CancelStaleWorkflowHandler { - public function __construct(private readonly EntityWorkflowRepository $workflowRepository, private EntityManagerInterface $em, private LoggerInterface $logger) + public function __construct(private readonly EntityWorkflowRepository $workflowRepository, private readonly Registry $registry, private EntityManagerInterface $em, private LoggerInterface $logger) { - } public function __invoke(CancelStaleWorkflow $message): void { - $workflowIds = $message->getWorkflowIds(); + $workflowId = $message->getWorkflowId(); - foreach ($workflowIds as $workflowId) { - $workflow = $this->workflowRepository->find($workflowId); + $workflow = $this->workflowRepository->find($workflowId); + $workflowComponent = $this->registry->get($workflow, $workflow->getWorkflowName()); + $metadataStore = $workflowComponent->getMetadataStore(); + $transitions = $workflowComponent->getEnabledTransitions($workflow); + $steps = $workflow->getSteps(); - $steps = $workflow->getSteps(); + $transitionApplied = false; + if (count($steps) === 1) { + $this->em->remove($workflow->getCurrentStep()); + $this->em->remove($workflow); + } else { + foreach ($transitions as $transition) { + $isFinal = $metadataStore->getMetadata('isFinal', $transition); + $isFinalPositive = $metadataStore->getMetadata('isFinalPositive', $transition); + if ($isFinal && !$isFinalPositive) { + $workflowComponent->apply($workflow, 'annule'); + $this->logger->info(sprintf('EntityWorkflow %d has been transitioned.', $workflowId)); + $transitionApplied = true; + break; + } + } + + if (!$transitionApplied) { + $this->logger->error(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId)); + throw new UnrecoverableMessageHandlingException( + sprintf('No valid transition found for EntityWorkflow %d.', $workflowId) + ); + } } + } } From 29fec50515e3daa1864913b8b4249198ef92f246 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Mon, 12 Aug 2024 11:45:27 +0200 Subject: [PATCH 35/60] Add cronjob and repository method to find and cancel stale workflows every other day --- .../Workflow/EntityWorkflowRepository.php | 13 +++++ .../Workflow/CancelStaleWorkflowCronJob.php | 55 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php diff --git a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php index f5696d2b9..b6e2e0814 100644 --- a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php +++ b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php @@ -198,6 +198,19 @@ class EntityWorkflowRepository implements ObjectRepository return $this->repository->findOneBy($criteria); } + public function findWorkflowsWithoutFinalStepAndOlderThan(\DateTimeImmutable $olderThanDate) + { + $qb = $this->repository->createQueryBuilder('sw'); + + $qb->select('sw.id') + ->where('NOT EXISTS (SELECT 1 FROM chill_main_entity_workflow_step ews WHERE ews.isFinal = TRUE AND ews.entityWorkflow = sw.id)') + ->andWhere(':olderThanDate > ALL (SELECT ews.transitionAt FROM chill_main_entity_workflow_step ews WHERE ews.transitionAt IS NOT NULL AND ews.entityWorkflow = sw.id)') + ->andWhere('sw.createdAt < :olderThanDate') + ->setParameter('olderThanDate', $olderThanDate); + + return $qb->getQuery()->getResult(); + } + public function getClassName(): string { return EntityWorkflow::class; diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php new file mode 100644 index 000000000..2114396f0 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php @@ -0,0 +1,55 @@ +clock->now() >= $cronJobExecution->getLastEnd()->add(new DateInterval('P1D')); + } + + public function getKey(): string + { + return self::KEY; + } + + public function run(array $lastExecutionData): ?array + { + $olderThanDate = $this->clock->now()->sub(new DateInterval(self::KEEP_INTERVAL)); + + $staleWorkflowIds = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate); + + $lastCanceled = self::LAST_CANCELED_WORKFLOW; + + foreach ($staleWorkflowIds as $wId) { + $this->messageBus->dispatch(new CancelStaleWorkflow($wId)); + $lastCanceled = $wId; + } + + return [self::LAST_CANCELED_WORKFLOW => $lastCanceled]; + } + +} From 6001bb6447e025f4d18fea1a2952295987932be5 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Mon, 12 Aug 2024 12:05:03 +0200 Subject: [PATCH 36/60] Add logger messages for possible debugging purposes --- .../Workflow/CancelStaleWorkflowCronJob.php | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php index 2114396f0..d8888efe8 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php @@ -6,22 +6,22 @@ use Chill\MainBundle\Cron\CronJobInterface; use Chill\MainBundle\Entity\CronJobExecution; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use DateInterval; -use Doctrine\DBAL\Connection; use Psr\Log\LoggerInterface; use Symfony\Component\Clock\ClockInterface; use Symfony\Component\Messenger\MessageBusInterface; class CancelStaleWorkflowCronJob implements CronJobInterface { - public const KEY = 'remove-stale-workflow'; + public const string KEY = 'remove-stale-workflow'; - public const KEEP_INTERVAL = 'P90D'; + public const string KEEP_INTERVAL = 'P90D'; - private const LAST_CANCELED_WORKFLOW = 'last-canceled-workflow-id'; + private const string LAST_CANCELED_WORKFLOW = 'last-canceled-workflow-id'; public function __construct(private readonly EntityWorkflowRepository $workflowRepository, private readonly ClockInterface $clock, private readonly MessageBusInterface $messageBus, + private readonly LoggerInterface $logger, ) { } @@ -38,18 +38,26 @@ class CancelStaleWorkflowCronJob implements CronJobInterface public function run(array $lastExecutionData): ?array { + $this->logger->info('Cronjob started: Canceling stale workflows.'); + $olderThanDate = $this->clock->now()->sub(new DateInterval(self::KEEP_INTERVAL)); - $staleWorkflowIds = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate); - $lastCanceled = self::LAST_CANCELED_WORKFLOW; + $processedCount = 0; foreach ($staleWorkflowIds as $wId) { - $this->messageBus->dispatch(new CancelStaleWorkflow($wId)); - $lastCanceled = $wId; + try { + $this->messageBus->dispatch(new CancelStaleWorkflow($wId)); + $lastCanceled = $wId; + $processedCount++; + } catch (\Exception $e) { + $this->logger->error("Failed to dispatch CancelStaleWorkflow for ID {$wId}", ['exception' => $e]); + continue; + } } + $this->logger->info("Cronjob completed: {$processedCount} workflows processed."); + return [self::LAST_CANCELED_WORKFLOW => $lastCanceled]; } - } From dab68fb4090c801bfe0ff4ad7ee0b8ac68f3228f Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Mon, 12 Aug 2024 12:05:31 +0200 Subject: [PATCH 37/60] Add CancelStaleWorkflowCronJobTest --- .../CancelStaleWorkflowCronJobTest.php | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php new file mode 100644 index 000000000..6b654f4df --- /dev/null +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php @@ -0,0 +1,107 @@ +connection = self::getContainer()->get(Connection::class); + } + + /** + * @dataProvider buildTestCanRunData + * @throws \Exception + */ + public function testCanRun(?CronJobExecution $cronJobExecution, bool $expected): void + { + $clock = new MockClock(new \DateTimeImmutable('2024-01-01 00:00:00', new \DateTimeZone('+00:00'))); + $logger = $this->createMock(LoggerInterface::class); + + $cronJob = new CancelStaleWorkflowCronJob($this->createMock(EntityWorkflowRepository::class), $clock, $this->buildMessageBus(), $logger); + + self::assertEquals($expected, $cronJob->canRun($cronJobExecution)); + } + + /** + * @throws \DateMalformedStringException + * @throws \DateInvalidTimeZoneException + * @throws Exception + */ + public function testRun(): void + { + $clock = new MockClock((new \DateTimeImmutable('now', new \DateTimeZone('+00:00')))->add(new \DateInterval('P120D'))); + $workflowRepository = $this->createMock(EntityWorkflowRepository::class); + $logger = $this->createMock(LoggerInterface::class); + + $workflowRepository->method('findWorkflowsWithoutFinalStepAndOlderThan')->willReturn([1, 2, 3]); + $messageBus = $this->buildMessageBus(true); + + $cronJob = new CancelStaleWorkflowCronJob($workflowRepository, $clock, $messageBus, $logger); + + $results = $cronJob->run([]); + + // Assert the result has the last canceled workflow ID + self::assertArrayHasKey('last-canceled-workflow-id', $results); + self::assertEquals(3, $results['last-canceled-workflow-id']); + } + + /** + * @throws \Exception + */ + public static function buildTestCanRunData(): iterable + { + yield [ + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-31 00:00:00', new \DateTimeZone('+00:00'))), + true, + ]; + + yield [ + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-30 23:59:59', new \DateTimeZone('+00:00'))), + true, + ]; + + yield [ + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-31 00:00:01', new \DateTimeZone('+00:00'))), + false, + ]; + } + + private function buildMessageBus(bool $expectDispatchAtLeastOnce = false): MessageBusInterface + { + $messageBus = $this->createMock(MessageBusInterface::class); + + $methodDispatch = match ($expectDispatchAtLeastOnce) { + true => $messageBus->expects($this->atLeastOnce())->method('dispatch')->with($this->isInstanceOf(CancelStaleWorkflow::class)), + false => $messageBus->method('dispatch'), + }; + + $methodDispatch->willReturnCallback(function (CancelStaleWorkflow $message) { + return new Envelope($message); + }); + + return $messageBus; + } +} From 35199b69934729e9956bcde41df501f014e74113 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Mon, 12 Aug 2024 15:49:22 +0200 Subject: [PATCH 38/60] Create test for CancelStaleWorkflowHandler: wip state --- .../CancelStaleWorkflowHandlerTest.php | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php new file mode 100644 index 000000000..8e2a5ec95 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php @@ -0,0 +1,126 @@ +createMock(EntityWorkflow::class); + $workflow->method('getSteps')->willReturn(new ArrayCollection([$this->createMock(EntityWorkflowStep::class)])); + $workflow->expects($this->once())->method('getCurrentStep')->willReturn(new EntityWorkflowStep()); + + $workflowRepository = $this->createMock(EntityWorkflowRepository::class); + $workflowRepository->expects($this->once())->method('find')->with($this->identicalTo(1))->willReturn($workflow); + + $em = $this->createMock(EntityManagerInterface::class); + $em->expects($this->exactly(2))->method('remove')->with($this->isInstanceOf(EntityWorkflowStep::class)); + $em->expects($this->once())->method('flush'); + + $registry = $this->createMock(Registry::class); + $logger = $this->createMock(LoggerInterface::class); + + $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); + + $handler(new CancelStaleWorkflow(1)); + } + + public function testInvokeWorkflowWithMultipleStepsAndValidTransition(): void + { + $workflow = $this->createMock(EntityWorkflow::class); + $workflow->method('getSteps')->willReturn(new ArrayCollection([$this->createMock(EntityWorkflowStep::class), $this->createMock(EntityWorkflowStep::class)])); + + $transition = $this->createMock(Transition::class); + + $workflowComponent = $this->createMock(WorkflowInterface::class); + $registryMock = $this->createMock(Registry::class); + $registryMock->method('get') + ->willReturn($workflowComponent); + + $workflowComponent->method('getEnabledTransitions')->willReturn([$transition]); + $workflowComponent->expects($this->once())->method('apply')->with($workflow, 'annule'); + + $metadataStore = $this->createMock(MetadataStore::class); + $metadataStore->method('getMetadata')->willReturnMap([ + ['isFinal', $transition, true], + ['isFinalPositive', $transition, false], + ]); + + $workflowComponent->method('getMetadataStore')->willReturn($metadataStore); + + $workflowRepository = $this->createMock(EntityWorkflowRepository::class); + $workflowRepository->expects($this->once())->method('find')->with($this->identicalTo(1))->willReturn($workflow); + + $em = $this->createMock(EntityManagerInterface::class); + $em->expects($this->never())->method('remove'); + $em->expects($this->once())->method('flush'); + + $registry = $this->createMock(Registry::class); + $registry->method('get')->willReturn($workflowComponent); + + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once())->method('info')->with('EntityWorkflow 1 has been transitioned.'); + + $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); + + $handler(new CancelStaleWorkflow(1)); + } + + public function testInvokeWorkflowWithMultipleStepsAndNoValidTransition(): void + { + $this->expectException(UnrecoverableMessageHandlingException::class); + $this->expectExceptionMessage('No valid transition found for EntityWorkflow 1.'); + + $workflow = $this->createMock(EntityWorkflow::class); + $workflow->method('getSteps')->willReturn(new ArrayCollection([$this->createMock(EntityWorkflowStep::class), $this->createMock(EntityWorkflowStep::class)])); + + $transition = $this->createMock(Transition::class); + + $workflowComponent = $this->createMock(WorkflowInterface::class); + $registryMock = $this->createMock(Registry::class); + $registryMock->method('get') + ->willReturn($workflowComponent); + $workflowComponent->method('getEnabledTransitions')->willReturn([$transition]); + + $metadataStore = $this->createMock(MetadataStoreInterface::class); + $metadataStore->method('getMetadata')->willReturnMap([ + ['isFinal', $transition, false], + ['isFinalPositive', $transition, true], + ]); + + $workflowComponent->method('getMetadataStore')->willReturn($metadataStore); + + $workflowRepository = $this->createMock(EntityWorkflowRepository::class); + $workflowRepository->expects($this->once())->method('find')->with($this->identicalTo(1))->willReturn($workflow); + + $em = $this->createMock(EntityManagerInterface::class); + $em->expects($this->never())->method('remove'); + $em->expects($this->never())->method('flush'); + + $registry = $this->createMock(Registry::class); + $registry->method('get')->willReturn($workflowComponent); + + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once())->method('error')->with('No valid transition found for EntityWorkflow 1.'); + + $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); + + $handler(new CancelStaleWorkflow(1)); + } +} From 5d84e997c1e5337f73cbd1061a424291a52deef5 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Mon, 12 Aug 2024 15:52:21 +0200 Subject: [PATCH 39/60] Php cs fixes --- .../Service/Workflow/CancelStaleWorkflow.php | 13 ++++++-- .../Workflow/CancelStaleWorkflowCronJob.php | 33 +++++++++++-------- .../Workflow/CancelStaleWorkflowHandler.php | 21 +++++++----- .../CancelStaleWorkflowCronJobTest.php | 6 ++++ .../CancelStaleWorkflowHandlerTest.php | 15 ++++++++- 5 files changed, 62 insertions(+), 26 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php index 078402855..118b9f57e 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php @@ -1,12 +1,19 @@ clock->now() >= $cronJobExecution->getLastEnd()->add(new DateInterval('P1D')); + return $this->clock->now() >= $cronJobExecution->getLastEnd()->add(new \DateInterval('P1D')); } public function getKey(): string @@ -40,7 +47,7 @@ class CancelStaleWorkflowCronJob implements CronJobInterface { $this->logger->info('Cronjob started: Canceling stale workflows.'); - $olderThanDate = $this->clock->now()->sub(new DateInterval(self::KEEP_INTERVAL)); + $olderThanDate = $this->clock->now()->sub(new \DateInterval(self::KEEP_INTERVAL)); $staleWorkflowIds = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate); $lastCanceled = self::LAST_CANCELED_WORKFLOW; $processedCount = 0; @@ -49,10 +56,10 @@ class CancelStaleWorkflowCronJob implements CronJobInterface try { $this->messageBus->dispatch(new CancelStaleWorkflow($wId)); $lastCanceled = $wId; - $processedCount++; + ++$processedCount; } catch (\Exception $e) { - $this->logger->error("Failed to dispatch CancelStaleWorkflow for ID {$wId}", ['exception' => $e]); - continue; + $this->logger->error("Failed to dispatch CancelStaleWorkflow for ID {$wId}", ['exception' => $e]); + continue; } } diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php index e35464f1d..533283a59 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php @@ -1,5 +1,14 @@ em->remove($workflow->getCurrentStep()); $this->em->remove($workflow); } else { @@ -46,12 +53,8 @@ class CancelStaleWorkflowHandler if (!$transitionApplied) { $this->logger->error(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId)); - throw new UnrecoverableMessageHandlingException( - sprintf('No valid transition found for EntityWorkflow %d.', $workflowId) - ); + throw new UnrecoverableMessageHandlingException(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId)); } } - } - } diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php index 6b654f4df..b56faf79b 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php @@ -23,6 +23,11 @@ use Symfony\Component\Clock\MockClock; use Symfony\Component\Messenger\Envelope; use Symfony\Component\Messenger\MessageBusInterface; +/** + * @internal + * + * @coversNothing + */ class CancelStaleWorkflowCronJobTest extends KernelTestCase { protected function setUp(): void @@ -33,6 +38,7 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase /** * @dataProvider buildTestCanRunData + * * @throws \Exception */ public function testCanRun(?CronJobExecution $cronJobExecution, bool $expected): void diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php index 8e2a5ec95..f7887a6cd 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php @@ -1,5 +1,14 @@ Date: Wed, 28 Aug 2024 11:52:01 +0200 Subject: [PATCH 40/60] Suffix message class with 'Message' and add check on workflow to assert no transitions were applied since message placed in queue --- .../Workflow/CancelStaleWorkflowCronJob.php | 4 +-- .../Workflow/CancelStaleWorkflowHandler.php | 27 +++++++++++++++---- ...low.php => CancelStaleWorkflowMessage.php} | 2 +- .../CancelStaleWorkflowCronJobTest.php | 24 ++++++++++------- .../CancelStaleWorkflowHandlerTest.php | 7 ++--- 5 files changed, 43 insertions(+), 21 deletions(-) rename src/Bundle/ChillMainBundle/Service/Workflow/{CancelStaleWorkflow.php => CancelStaleWorkflowMessage.php} (92%) diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php index cb6f1c971..786bae3c9 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php @@ -54,8 +54,8 @@ class CancelStaleWorkflowCronJob implements CronJobInterface foreach ($staleWorkflowIds as $wId) { try { - $this->messageBus->dispatch(new CancelStaleWorkflow($wId)); - $lastCanceled = $wId; + $this->messageBus->dispatch(new CancelStaleWorkflowMessage($wId)); + $lastCanceled = max($wId, $lastCanceled); ++$processedCount; } catch (\Exception $e) { $this->logger->error("Failed to dispatch CancelStaleWorkflow for ID {$wId}", ['exception' => $e]); diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php index 533283a59..a3eefb193 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php @@ -14,6 +14,7 @@ namespace Chill\MainBundle\Service\Workflow; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; +use Symfony\Component\Clock\ClockInterface; use Symfony\Component\Messenger\Attribute\AsMessageHandler; use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException; use Symfony\Component\Workflow\Registry; @@ -21,13 +22,29 @@ use Symfony\Component\Workflow\Registry; #[AsMessageHandler] class CancelStaleWorkflowHandler { - public function __construct(private readonly EntityWorkflowRepository $workflowRepository, private readonly Registry $registry, private EntityManagerInterface $em, private LoggerInterface $logger) {} + public const string KEEP_INTERVAL = 'P90D'; - public function __invoke(CancelStaleWorkflow $message): void + public function __construct(private readonly EntityWorkflowRepository $workflowRepository, private readonly Registry $registry, private readonly EntityManagerInterface $em, private readonly LoggerInterface $logger, private readonly ClockInterface $clock) {} + + public function __invoke(CancelStaleWorkflowMessage $message): void { $workflowId = $message->getWorkflowId(); + $olderThanDate = $this->clock->now()->sub(new \DateInterval(self::KEEP_INTERVAL)); + $staleWorkflows = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate); + $workflow = $this->workflowRepository->find($workflowId); + + if (in_array($workflow, $staleWorkflows, true)) { + $this->logger->alert('Workflow has transitioned in the meantime.', [$workflowId]); + return; + } + + if (null === $workflow) { + $this->logger->alert('Workflow was not found!', [$workflowId]); + return; + } + $workflowComponent = $this->registry->get($workflow, $workflow->getWorkflowName()); $metadataStore = $workflowComponent->getMetadataStore(); $transitions = $workflowComponent->getEnabledTransitions($workflow); @@ -44,15 +61,15 @@ class CancelStaleWorkflowHandler $isFinalPositive = $metadataStore->getMetadata('isFinalPositive', $transition); if ($isFinal && !$isFinalPositive) { - $workflowComponent->apply($workflow, 'annule'); - $this->logger->info(sprintf('EntityWorkflow %d has been transitioned.', $workflowId)); + $workflowComponent->apply($workflow, $transition->getName()); + $this->logger->info('EntityWorkflow has been cancelled automatically.', [$workflowId]); $transitionApplied = true; break; } } if (!$transitionApplied) { - $this->logger->error(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId)); + $this->logger->error('No valid transition found for EntityWorkflow %d.', [$workflowId]); throw new UnrecoverableMessageHandlingException(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId)); } } diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowMessage.php similarity index 92% rename from src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php rename to src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowMessage.php index 118b9f57e..30d2b6ab8 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowMessage.php @@ -11,7 +11,7 @@ declare(strict_types=1); namespace Chill\MainBundle\Service\Workflow; -class CancelStaleWorkflow +class CancelStaleWorkflowMessage { public function __construct(public int $workflowId) {} diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php index b56faf79b..f433e8b48 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php @@ -15,6 +15,10 @@ use Chill\MainBundle\Entity\CronJobExecution; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflow; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowCronJob; +use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowMessage; +use DateInvalidTimeZoneException; +use DateMalformedStringException; +use DateTimeImmutable; use Doctrine\DBAL\Connection; use PHPUnit\Framework\MockObject\Exception; use Psr\Log\LoggerInterface; @@ -43,7 +47,7 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase */ public function testCanRun(?CronJobExecution $cronJobExecution, bool $expected): void { - $clock = new MockClock(new \DateTimeImmutable('2024-01-01 00:00:00', new \DateTimeZone('+00:00'))); + $clock = new MockClock(new DateTimeImmutable('2024-01-01 00:00:00', new \DateTimeZone('+00:00'))); $logger = $this->createMock(LoggerInterface::class); $cronJob = new CancelStaleWorkflowCronJob($this->createMock(EntityWorkflowRepository::class), $clock, $this->buildMessageBus(), $logger); @@ -52,17 +56,17 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase } /** - * @throws \DateMalformedStringException - * @throws \DateInvalidTimeZoneException - * @throws Exception + * @throws DateMalformedStringException + * @throws DateInvalidTimeZoneException + * @throws \Exception|Exception */ public function testRun(): void { - $clock = new MockClock((new \DateTimeImmutable('now', new \DateTimeZone('+00:00')))->add(new \DateInterval('P120D'))); + $clock = new MockClock((new DateTimeImmutable('now', new \DateTimeZone('+00:00')))->add(new \DateInterval('P120D'))); $workflowRepository = $this->createMock(EntityWorkflowRepository::class); $logger = $this->createMock(LoggerInterface::class); - $workflowRepository->method('findWorkflowsWithoutFinalStepAndOlderThan')->willReturn([1, 2, 3]); + $workflowRepository->method('findWorkflowsWithoutFinalStepAndOlderThan')->willReturn([1, 3, 2]); $messageBus = $this->buildMessageBus(true); $cronJob = new CancelStaleWorkflowCronJob($workflowRepository, $clock, $messageBus, $logger); @@ -80,17 +84,17 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase public static function buildTestCanRunData(): iterable { yield [ - (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-31 00:00:00', new \DateTimeZone('+00:00'))), + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new DateTimeImmutable('2023-12-31 00:00:00', new \DateTimeZone('+00:00'))), true, ]; yield [ - (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-30 23:59:59', new \DateTimeZone('+00:00'))), + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new DateTimeImmutable('2023-12-30 23:59:59', new \DateTimeZone('+00:00'))), true, ]; yield [ - (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-31 00:00:01', new \DateTimeZone('+00:00'))), + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new DateTimeImmutable('2023-12-31 00:00:01', new \DateTimeZone('+00:00'))), false, ]; } @@ -104,7 +108,7 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase false => $messageBus->method('dispatch'), }; - $methodDispatch->willReturnCallback(function (CancelStaleWorkflow $message) { + $methodDispatch->willReturnCallback(function (CancelStaleWorkflowMessage $message) { return new Envelope($message); }); diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php index f7887a6cd..dbf88098f 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php @@ -16,6 +16,7 @@ use Chill\MainBundle\Entity\Workflow\EntityWorkflowStep; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflow; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowHandler; +use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowMessage; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\TestCase; @@ -51,7 +52,7 @@ class CancelStaleWorkflowHandlerTest extends TestCase $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); - $handler(new CancelStaleWorkflow(1)); + $handler(new CancelStaleWorkflowMessage(1)); } public function testInvokeWorkflowWithMultipleStepsAndValidTransition(): void @@ -92,7 +93,7 @@ class CancelStaleWorkflowHandlerTest extends TestCase $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); - $handler(new CancelStaleWorkflow(1)); + $handler(new CancelStaleWorkflowMessage(1)); } public function testInvokeWorkflowWithMultipleStepsAndNoValidTransition(): void @@ -134,6 +135,6 @@ class CancelStaleWorkflowHandlerTest extends TestCase $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); - $handler(new CancelStaleWorkflow(1)); + $handler(new CancelStaleWorkflowMessage(1)); } } From 2e69d2df90ddac5a55942874fef87619938b18fb Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 28 Aug 2024 14:27:27 +0200 Subject: [PATCH 41/60] Adjust test to work with actual workflow + minor fix of handler logic --- .../Entity/Workflow/EntityWorkflow.php | 2 +- .../Workflow/CancelStaleWorkflowHandler.php | 4 +- .../CancelStaleWorkflowHandlerTest.php | 168 ++++++++---------- 3 files changed, 76 insertions(+), 98 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php index 1f32a1985..6e6cee077 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php @@ -38,7 +38,7 @@ class EntityWorkflow implements TrackCreationInterface, TrackUpdateInterface /** * @var Collection */ - #[ORM\OneToMany(targetEntity: EntityWorkflowComment::class, mappedBy: 'entityWorkflow', orphanRemoval: true)] + #[ORM\OneToMany(mappedBy: 'entityWorkflow', targetEntity: EntityWorkflowComment::class, orphanRemoval: true)] private Collection $comments; #[ORM\Id] diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php index a3eefb193..2aae89f3c 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php @@ -35,7 +35,7 @@ class CancelStaleWorkflowHandler $workflow = $this->workflowRepository->find($workflowId); - if (in_array($workflow, $staleWorkflows, true)) { + if (!in_array($workflow, $staleWorkflows, true)) { $this->logger->alert('Workflow has transitioned in the meantime.', [$workflowId]); return; } @@ -69,7 +69,7 @@ class CancelStaleWorkflowHandler } if (!$transitionApplied) { - $this->logger->error('No valid transition found for EntityWorkflow %d.', [$workflowId]); + $this->logger->error('No valid transition found for EntityWorkflow.', [$workflowId]); throw new UnrecoverableMessageHandlingException(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId)); } } diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php index dbf88098f..23c99d16a 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php @@ -2,139 +2,117 @@ declare(strict_types=1); -/* - * Chill is a software for social workers - * - * For the full copyright and license information, please view - * the LICENSE file that was distributed with this source code. - */ - namespace Services\Workflow; use Chill\MainBundle\Entity\Workflow\EntityWorkflow; use Chill\MainBundle\Entity\Workflow\EntityWorkflowStep; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; -use Chill\MainBundle\Service\Workflow\CancelStaleWorkflow; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowHandler; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowMessage; -use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; -use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException; -use Symfony\Component\Workflow\Metadata\MetadataStoreInterface; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use Symfony\Component\Clock\ClockInterface; use Symfony\Component\Workflow\Registry; -use Symfony\Component\Workflow\Transition; use Symfony\Component\Workflow\WorkflowInterface; +use Symfony\Component\Yaml\Yaml; +use DateTimeImmutable; -/** - * @internal - * - * @coversNothing - */ -class CancelStaleWorkflowHandlerTest extends TestCase +class CancelStaleWorkflowHandlerTest extends KernelTestCase { - public function testInvokeWorkflowWithOneStep(): void + private EntityManagerInterface $em; + private Registry $registry; + private LoggerInterface $logger; + private EntityWorkflowRepository $workflowRepository; + private ClockInterface $clock; + private string $workflowName; + + protected function setUp(): void { - $workflow = $this->createMock(EntityWorkflow::class); - $workflow->method('getSteps')->willReturn(new ArrayCollection([$this->createMock(EntityWorkflowStep::class)])); - $workflow->expects($this->once())->method('getCurrentStep')->willReturn(new EntityWorkflowStep()); + // Boot the Symfony kernel + self::bootKernel(); - $workflowRepository = $this->createMock(EntityWorkflowRepository::class); - $workflowRepository->expects($this->once())->method('find')->with($this->identicalTo(1))->willReturn($workflow); + // Get the actual services from the container + $this->em = self::getContainer()->get(EntityManagerInterface::class); + $this->registry = self::getContainer()->get(Registry::class); + $this->logger = self::getContainer()->get(LoggerInterface::class); + $this->clock = self::getContainer()->get(ClockInterface::class); + $this->workflowRepository = $this->createMock(EntityWorkflowRepository::class); - $em = $this->createMock(EntityManagerInterface::class); - $em->expects($this->exactly(2))->method('remove')->with($this->isInstanceOf(EntityWorkflowStep::class)); - $em->expects($this->once())->method('flush'); + // Retrieve the workflow configuration dynamically + $configPath = self::$kernel->getProjectDir() . '/config/packages/workflow.yaml'; // Adjust the path if needed + $config = Yaml::parseFile($configPath); - $registry = $this->createMock(Registry::class); - $logger = $this->createMock(LoggerInterface::class); - - $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); - - $handler(new CancelStaleWorkflowMessage(1)); + // Extract the workflow name from the configuration + $this->workflowName = array_key_first($config['framework']['workflows']); } - public function testInvokeWorkflowWithMultipleStepsAndValidTransition(): void + public function testWorkflowWithOneStepOlderThan90DaysIsDeleted(): void { - $workflow = $this->createMock(EntityWorkflow::class); - $workflow->method('getSteps')->willReturn(new ArrayCollection([$this->createMock(EntityWorkflowStep::class), $this->createMock(EntityWorkflowStep::class)])); + $workflow = new EntityWorkflow(); + $initialStep = new EntityWorkflowStep(); + $initialStep->setTransitionAt(new DateTimeImmutable('-93 days')); + $workflow->addStep($initialStep); - $transition = $this->createMock(Transition::class); + $this->em->persist($workflow); + $this->em->flush(); - $workflowComponent = $this->createMock(WorkflowInterface::class); - $registryMock = $this->createMock(Registry::class); - $registryMock->method('get') - ->willReturn($workflowComponent); + $this->handleStaleWorkflow($workflow); - $workflowComponent->method('getEnabledTransitions')->willReturn([$transition]); - $workflowComponent->expects($this->once())->method('apply')->with($workflow, 'annule'); + $deletedWorkflow = $this->workflowRepository->find($workflow->getId()); + $this->assertNull($deletedWorkflow, 'The workflow should be deleted.'); - $metadataStore = $this->createMock(MetadataStore::class); - $metadataStore->method('getMetadata')->willReturnMap([ - ['isFinal', $transition, true], - ['isFinalPositive', $transition, false], - ]); - - $workflowComponent->method('getMetadataStore')->willReturn($metadataStore); - - $workflowRepository = $this->createMock(EntityWorkflowRepository::class); - $workflowRepository->expects($this->once())->method('find')->with($this->identicalTo(1))->willReturn($workflow); - - $em = $this->createMock(EntityManagerInterface::class); - $em->expects($this->never())->method('remove'); - $em->expects($this->once())->method('flush'); - - $registry = $this->createMock(Registry::class); - $registry->method('get')->willReturn($workflowComponent); - - $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->once())->method('info')->with('EntityWorkflow 1 has been transitioned.'); - - $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); - - $handler(new CancelStaleWorkflowMessage(1)); + $this->assertNull($this->em->getRepository(EntityWorkflowStep::class)->find($initialStep->getId()), 'The workflow step should be deleted.'); } - public function testInvokeWorkflowWithMultipleStepsAndNoValidTransition(): void + public function testWorkflowWithMultipleStepsAndNoRecentTransitionsIsCanceled(): void { - $this->expectException(UnrecoverableMessageHandlingException::class); - $this->expectExceptionMessage('No valid transition found for EntityWorkflow 1.'); + $workflow = new EntityWorkflow(); + $step1 = new EntityWorkflowStep(); + $step2 = new EntityWorkflowStep(); - $workflow = $this->createMock(EntityWorkflow::class); - $workflow->method('getSteps')->willReturn(new ArrayCollection([$this->createMock(EntityWorkflowStep::class), $this->createMock(EntityWorkflowStep::class)])); + $step1->setTransitionAt(new DateTimeImmutable('-92 days')); + $step2->setTransitionAt(new DateTimeImmutable('-91 days')); - $transition = $this->createMock(Transition::class); + $workflow->addStep($step1); + $workflow->addStep($step2); - $workflowComponent = $this->createMock(WorkflowInterface::class); - $registryMock = $this->createMock(Registry::class); - $registryMock->method('get') - ->willReturn($workflowComponent); - $workflowComponent->method('getEnabledTransitions')->willReturn([$transition]); + $this->em->persist($workflow); + $this->em->flush(); - $metadataStore = $this->createMock(MetadataStoreInterface::class); - $metadataStore->method('getMetadata')->willReturnMap([ - ['isFinal', $transition, false], - ['isFinalPositive', $transition, true], - ]); + /** @var WorkflowInterface $workflowComponent */ + $workflowComponent = $this->registry->get($workflow, $this->workflowName); - $workflowComponent->method('getMetadataStore')->willReturn($metadataStore); + $transitions = $workflowComponent->getEnabledTransitions($workflow); + $metadataStore = $workflowComponent->getMetadataStore(); - $workflowRepository = $this->createMock(EntityWorkflowRepository::class); - $workflowRepository->expects($this->once())->method('find')->with($this->identicalTo(1))->willReturn($workflow); + $expectedTransition = null; - $em = $this->createMock(EntityManagerInterface::class); - $em->expects($this->never())->method('remove'); - $em->expects($this->never())->method('flush'); + // Find the transition that was expected to be applied by the handler + foreach ($transitions as $transition) { + $isFinal = $metadataStore->getMetadata('isFinal', $transition); + $isFinalPositive = $metadataStore->getMetadata('isFinalPositive', $transition); - $registry = $this->createMock(Registry::class); - $registry->method('get')->willReturn($workflowComponent); + if ($isFinal === true && $isFinalPositive === false) { + $expectedTransition = $transition; + break; + } + } - $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->once())->method('error')->with('No valid transition found for EntityWorkflow 1.'); + $this->assertNotNull($expectedTransition, 'Expected to find a valid transition with isFinal = true and isFinalPositive = false.'); - $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); + $this->handleStaleWorkflow($workflow); + $updatedWorkflow = $this->workflowRepository->find($workflow->getId()); + + $this->assertEquals($expectedTransition->getName(), $updatedWorkflow->getCurrentStep()); - $handler(new CancelStaleWorkflowMessage(1)); } + + public function handleStaleWorkflow($workflow): void + { + $handler = new CancelStaleWorkflowHandler($this->workflowRepository, $this->registry, $this->em, $this->logger, $this->clock); + $handler(new CancelStaleWorkflowMessage($workflow->getId())); + } + } From b97eabf0d28e48f61bac510783fa238caef7fbf6 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Thu, 29 Aug 2024 16:51:25 +0200 Subject: [PATCH 42/60] Get workflowComponent directly from registry --- .../Workflow/CancelStaleWorkflowHandlerTest.php | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php index 23c99d16a..2c3016f82 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php @@ -26,7 +26,6 @@ class CancelStaleWorkflowHandlerTest extends KernelTestCase private LoggerInterface $logger; private EntityWorkflowRepository $workflowRepository; private ClockInterface $clock; - private string $workflowName; protected function setUp(): void { @@ -39,13 +38,6 @@ class CancelStaleWorkflowHandlerTest extends KernelTestCase $this->logger = self::getContainer()->get(LoggerInterface::class); $this->clock = self::getContainer()->get(ClockInterface::class); $this->workflowRepository = $this->createMock(EntityWorkflowRepository::class); - - // Retrieve the workflow configuration dynamically - $configPath = self::$kernel->getProjectDir() . '/config/packages/workflow.yaml'; // Adjust the path if needed - $config = Yaml::parseFile($configPath); - - // Extract the workflow name from the configuration - $this->workflowName = array_key_first($config['framework']['workflows']); } public function testWorkflowWithOneStepOlderThan90DaysIsDeleted(): void @@ -82,7 +74,7 @@ class CancelStaleWorkflowHandlerTest extends KernelTestCase $this->em->flush(); /** @var WorkflowInterface $workflowComponent */ - $workflowComponent = $this->registry->get($workflow, $this->workflowName); + $workflowComponent = $this->registry->get($workflowComponent); $transitions = $workflowComponent->getEnabledTransitions($workflow); $metadataStore = $workflowComponent->getMetadataStore(); From ee9530d03f88c15c35d107036b73dc34eb66a536 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 9 Sep 2024 10:40:05 +0200 Subject: [PATCH 43/60] More conditions to find staled workflows --- .../Workflow/EntityWorkflowRepository.php | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php index b6e2e0814..62282fbe9 100644 --- a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php +++ b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php @@ -198,13 +198,28 @@ class EntityWorkflowRepository implements ObjectRepository return $this->repository->findOneBy($criteria); } - public function findWorkflowsWithoutFinalStepAndOlderThan(\DateTimeImmutable $olderThanDate) + /** + * Finds workflows that are not finalized and are older than the specified date. + * + * @param \DateTimeImmutable $olderThanDate the date to compare against + * + * @return list the list of workflow IDs that meet the criteria + */ + public function findWorkflowsWithoutFinalStepAndOlderThan(\DateTimeImmutable $olderThanDate): array { $qb = $this->repository->createQueryBuilder('sw'); $qb->select('sw.id') + // only the workflow which are not finalized ->where('NOT EXISTS (SELECT 1 FROM chill_main_entity_workflow_step ews WHERE ews.isFinal = TRUE AND ews.entityWorkflow = sw.id)') - ->andWhere(':olderThanDate > ALL (SELECT ews.transitionAt FROM chill_main_entity_workflow_step ews WHERE ews.transitionAt IS NOT NULL AND ews.entityWorkflow = sw.id)') + ->andWhere( + $qb->expr()->orX( + // only the workflow where all the last transition is older than transitionAt + ':olderThanDate > ALL (SELECT ews.transitionAt FROM chill_main_entity_workflow_step ews WHERE ews.transitionAt IS NOT NULL AND ews.entityWorkflow = sw.id)', + // or the workflow which have only the initial step, with no transition + '1 = (SELECT COUNT(ews.id) FROM chill_main_entity_workflow_step ews WHERE ews.step = :initial AND ews.transitionAt IS NULL AND ews.createdAt < :olderThanDate AND ews.entityWorkflow = sw.id)', + ) + ) ->andWhere('sw.createdAt < :olderThanDate') ->setParameter('olderThanDate', $olderThanDate); From d152efe084aa4d3455d2ae9c928128ec4d560deb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 9 Sep 2024 14:58:19 +0200 Subject: [PATCH 44/60] Refactor imports and remove redundant type strings This commit refactors the usage of \DateTimeImmutable to ensure consistent namespacing and removes unnecessary string type declarations from constants in CancelStaleWorkflowCronJob. These changes improve code readability and maintainability. --- .../Workflow/CancelStaleWorkflowCronJob.php | 6 +++--- .../Workflow/CancelStaleWorkflowCronJobTest.php | 17 +++++++---------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php index 786bae3c9..767af2c5c 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php @@ -20,11 +20,11 @@ use Symfony\Component\Messenger\MessageBusInterface; class CancelStaleWorkflowCronJob implements CronJobInterface { - public const string KEY = 'remove-stale-workflow'; + public const KEY = 'remove-stale-workflow'; - public const string KEEP_INTERVAL = 'P90D'; + public const KEEP_INTERVAL = 'P90D'; - private const string LAST_CANCELED_WORKFLOW = 'last-canceled-workflow-id'; + private const LAST_CANCELED_WORKFLOW = 'last-canceled-workflow-id'; public function __construct( private readonly EntityWorkflowRepository $workflowRepository, diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php index f433e8b48..8e06735f6 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php @@ -16,9 +16,6 @@ use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflow; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowCronJob; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowMessage; -use DateInvalidTimeZoneException; -use DateMalformedStringException; -use DateTimeImmutable; use Doctrine\DBAL\Connection; use PHPUnit\Framework\MockObject\Exception; use Psr\Log\LoggerInterface; @@ -47,7 +44,7 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase */ public function testCanRun(?CronJobExecution $cronJobExecution, bool $expected): void { - $clock = new MockClock(new DateTimeImmutable('2024-01-01 00:00:00', new \DateTimeZone('+00:00'))); + $clock = new MockClock(new \DateTimeImmutable('2024-01-01 00:00:00', new \DateTimeZone('+00:00'))); $logger = $this->createMock(LoggerInterface::class); $cronJob = new CancelStaleWorkflowCronJob($this->createMock(EntityWorkflowRepository::class), $clock, $this->buildMessageBus(), $logger); @@ -56,13 +53,13 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase } /** - * @throws DateMalformedStringException - * @throws DateInvalidTimeZoneException + * @throws \DateMalformedStringException + * @throws \DateInvalidTimeZoneException * @throws \Exception|Exception */ public function testRun(): void { - $clock = new MockClock((new DateTimeImmutable('now', new \DateTimeZone('+00:00')))->add(new \DateInterval('P120D'))); + $clock = new MockClock((new \DateTimeImmutable('now', new \DateTimeZone('+00:00')))->add(new \DateInterval('P120D'))); $workflowRepository = $this->createMock(EntityWorkflowRepository::class); $logger = $this->createMock(LoggerInterface::class); @@ -84,17 +81,17 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase public static function buildTestCanRunData(): iterable { yield [ - (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new DateTimeImmutable('2023-12-31 00:00:00', new \DateTimeZone('+00:00'))), + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-31 00:00:00', new \DateTimeZone('+00:00'))), true, ]; yield [ - (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new DateTimeImmutable('2023-12-30 23:59:59', new \DateTimeZone('+00:00'))), + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-30 23:59:59', new \DateTimeZone('+00:00'))), true, ]; yield [ - (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new DateTimeImmutable('2023-12-31 00:00:01', new \DateTimeZone('+00:00'))), + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-31 00:00:01', new \DateTimeZone('+00:00'))), false, ]; } From f4356ac249ef91bb93c8beb04700d11f6a2250e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 9 Sep 2024 14:59:26 +0200 Subject: [PATCH 45/60] Add test for detecting stale workflows and enhance handler Added a new test to check if workflows are stale in EntityWorkflowTest. Enhanced CancelStaleWorkflowHandler to handle stale workflows more accurately, including checking if workflows have transitioned recently. Updated EntityWorkflow entity to cascade remove workflow steps. Refactor tests for handler, to avoid using $kernel during tests --- .../Entity/Workflow/EntityWorkflow.php | 34 ++- .../Workflow/CancelStaleWorkflowHandler.php | 75 ++++--- .../Entity/Workflow/EntityWorkflowTest.php | 27 +++ .../CancelStaleWorkflowCronJobTest.php | 4 +- .../CancelStaleWorkflowHandlerTest.php | 193 +++++++++++------- 5 files changed, 226 insertions(+), 107 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php index 6e6cee077..08b5ee41f 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php @@ -56,7 +56,7 @@ class EntityWorkflow implements TrackCreationInterface, TrackUpdateInterface * @var Collection&Selectable */ #[Assert\Valid(traverse: true)] - #[ORM\OneToMany(mappedBy: 'entityWorkflow', targetEntity: EntityWorkflowStep::class, cascade: ['persist'], orphanRemoval: true)] + #[ORM\OneToMany(mappedBy: 'entityWorkflow', targetEntity: EntityWorkflowStep::class, cascade: ['persist', 'remove'], orphanRemoval: true)] #[ORM\OrderBy(['transitionAt' => \Doctrine\Common\Collections\Criteria::ASC, 'id' => 'ASC'])] private Collection&Selectable $steps; @@ -488,4 +488,36 @@ class EntityWorkflow implements TrackCreationInterface, TrackUpdateInterface { return $this->getCurrentStep()->getHoldsOnStep()->count() > 0; } + + /** + * Determines if the workflow has become stale after a given date. + * + * This function checks the creation date and the transition states of the workflow steps. + * A workflow is considered stale if: + * - The creation date is before the given date and no transitions have occurred since the creation. + * - Or if there are no transitions after the given date. + * + * @param \DateTimeImmutable $at the date to compare against the workflow's status + * + * @return bool true if the workflow is stale after the given date, false otherwise + */ + public function isStaledAt(\DateTimeImmutable $at): bool + { + // if there is no transition since the creation, then the workflow is staled + if ('initial' === $this->getCurrentStep()->getCurrentStep() + && null === $this->getCurrentStep()->getTransitionAt() + ) { + if (null === $this->getCreatedAt()) { + return false; + } + + if ($this->getCreatedAt() < $at) { + return true; + } + + return false; + } + + return $this->getCurrentStepChained()->getPrevious()->getTransitionAt() < $at; + } } diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php index 2aae89f3c..54485dad5 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php @@ -12,6 +12,7 @@ declare(strict_types=1); namespace Chill\MainBundle\Service\Workflow; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; +use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Symfony\Component\Clock\ClockInterface; @@ -20,58 +21,68 @@ use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException; use Symfony\Component\Workflow\Registry; #[AsMessageHandler] -class CancelStaleWorkflowHandler +final readonly class CancelStaleWorkflowHandler { - public const string KEEP_INTERVAL = 'P90D'; - - public function __construct(private readonly EntityWorkflowRepository $workflowRepository, private readonly Registry $registry, private readonly EntityManagerInterface $em, private readonly LoggerInterface $logger, private readonly ClockInterface $clock) {} + public function __construct( + private EntityWorkflowRepository $workflowRepository, + private Registry $registry, + private EntityManagerInterface $em, + private LoggerInterface $logger, + private ClockInterface $clock, + ) {} public function __invoke(CancelStaleWorkflowMessage $message): void { $workflowId = $message->getWorkflowId(); + $olderThanDate = $this->clock->now()->sub(new \DateInterval(CancelStaleWorkflowCronJob::KEEP_INTERVAL)); - $olderThanDate = $this->clock->now()->sub(new \DateInterval(self::KEEP_INTERVAL)); - $staleWorkflows = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate); + $workflow = $this->workflowRepository->find($message->getWorkflowId()); + if (null === $workflow) { + $this->logger->alert('Workflow was not found!', [$workflowId]); - $workflow = $this->workflowRepository->find($workflowId); - - if (!in_array($workflow, $staleWorkflows, true)) { - $this->logger->alert('Workflow has transitioned in the meantime.', [$workflowId]); return; } - if (null === $workflow) { - $this->logger->alert('Workflow was not found!', [$workflowId]); - return; + if (false === $workflow->isStaledAt($olderThanDate)) { + $this->logger->alert('Workflow has transitioned in the meantime.', [$workflowId]); + + throw new UnrecoverableMessageHandlingException('the workflow is not staled any more'); } $workflowComponent = $this->registry->get($workflow, $workflow->getWorkflowName()); $metadataStore = $workflowComponent->getMetadataStore(); $transitions = $workflowComponent->getEnabledTransitions($workflow); - $steps = $workflow->getSteps(); $transitionApplied = false; + $wasInInitialPosition = 'initial' === $workflow->getStep(); - if (1 === count($steps)) { - $this->em->remove($workflow->getCurrentStep()); - $this->em->remove($workflow); - } else { - foreach ($transitions as $transition) { - $isFinal = $metadataStore->getMetadata('isFinal', $transition); - $isFinalPositive = $metadataStore->getMetadata('isFinalPositive', $transition); + foreach ($transitions as $transition) { + $isFinal = $metadataStore->getMetadata('isFinal', $transition); + $isFinalPositive = $metadataStore->getMetadata('isFinalPositive', $transition); - if ($isFinal && !$isFinalPositive) { - $workflowComponent->apply($workflow, $transition->getName()); - $this->logger->info('EntityWorkflow has been cancelled automatically.', [$workflowId]); - $transitionApplied = true; - break; - } - } - - if (!$transitionApplied) { - $this->logger->error('No valid transition found for EntityWorkflow.', [$workflowId]); - throw new UnrecoverableMessageHandlingException(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId)); + if ($isFinal && !$isFinalPositive) { + $dto = new WorkflowTransitionContextDTO($workflow); + $workflowComponent->apply($workflow, $transition->getName(), [ + 'context' => $dto, + 'byUser' => null, + 'transitionAt' => $this->clock->now(), + 'transition' => $transition->getName(), + ]); + $this->logger->info('EntityWorkflow has been cancelled automatically.', [$workflowId]); + $transitionApplied = true; + break; } } + + if (!$transitionApplied) { + $this->logger->error('No valid transition found for EntityWorkflow.', [$workflowId]); + throw new UnrecoverableMessageHandlingException(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId)); + } + + if ($wasInInitialPosition) { + $this->em->remove($workflow); + } + + $this->em->flush(); } } diff --git a/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php b/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php index 4eb56f995..3241a41fc 100644 --- a/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php @@ -138,4 +138,31 @@ final class EntityWorkflowTest extends TestCase self::assertContains($person1, $persons); self::assertContains($person2, $persons); } + + public function testIsStaledAt(): void + { + $creationDate = new \DateTimeImmutable('2024-01-01'); + $firstStepDate = new \DateTimeImmutable('2024-01-02'); + $afterFistStep = new \DateTimeImmutable('2024-01-03'); + + $entityWorkflow = new EntityWorkflow(); + + self::assertFalse($entityWorkflow->isStaledAt($creationDate), 'an entityWorkflow with null createdAt date should never be staled at initial step'); + self::assertFalse($entityWorkflow->isStaledAt($firstStepDate), 'an entityWorkflow with null createdAt date should never be staled at initial step'); + self::assertFalse($entityWorkflow->isStaledAt($afterFistStep), 'an entityWorkflow with null createdAt date should never be staled at initial step'); + + $entityWorkflow->setCreatedAt($creationDate); + + self::assertFalse($entityWorkflow->isStaledAt($creationDate), 'an entityWorkflow with no step after initial should be staled'); + self::assertTrue($entityWorkflow->isStaledAt($firstStepDate), 'an entityWorkflow with no step after initial should be staled'); + self::assertTrue($entityWorkflow->isStaledAt($afterFistStep), 'an entityWorkflow with no step after initial should be staled'); + + // apply a first step + $dto = new WorkflowTransitionContextDTO($entityWorkflow); + $entityWorkflow->setStep('new_step', $dto, 'to_new_step', $firstStepDate); + + self::assertFalse($entityWorkflow->isStaledAt($creationDate)); + self::assertFalse($entityWorkflow->isStaledAt($firstStepDate)); + self::assertTrue($entityWorkflow->isStaledAt($afterFistStep)); + } } diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php index 8e06735f6..07bca6bc5 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php @@ -105,9 +105,7 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase false => $messageBus->method('dispatch'), }; - $methodDispatch->willReturnCallback(function (CancelStaleWorkflowMessage $message) { - return new Envelope($message); - }); + $methodDispatch->willReturnCallback(fn (CancelStaleWorkflowMessage $message) => new Envelope($message)); return $messageBus; } diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php index 2c3016f82..b6cac68de 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php @@ -2,109 +2,160 @@ declare(strict_types=1); +/* + * Chill is a software for social workers + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + namespace Services\Workflow; +use Chill\MainBundle\Entity\User; use Chill\MainBundle\Entity\Workflow\EntityWorkflow; -use Chill\MainBundle\Entity\Workflow\EntityWorkflowStep; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowHandler; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowMessage; +use Chill\MainBundle\Workflow\EntityWorkflowMarkingStore; +use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO; use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\TestCase; -use Psr\Log\LoggerInterface; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use Prophecy\PhpUnit\ProphecyTrait; +use Psr\Log\NullLogger; use Symfony\Component\Clock\ClockInterface; +use Symfony\Component\Clock\MockClock; +use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException; +use Symfony\Component\Workflow\DefinitionBuilder; +use Symfony\Component\Workflow\Metadata\InMemoryMetadataStore; 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; -use Symfony\Component\Yaml\Yaml; -use DateTimeImmutable; -class CancelStaleWorkflowHandlerTest extends KernelTestCase +/** + * @internal + * + * @coversNothing + */ +class CancelStaleWorkflowHandlerTest extends TestCase { - private EntityManagerInterface $em; - private Registry $registry; - private LoggerInterface $logger; - private EntityWorkflowRepository $workflowRepository; - private ClockInterface $clock; + use ProphecyTrait; - protected function setUp(): void + public function testWorkflowWithOneStepOlderThan90DaysIsCanceled(): void { - // Boot the Symfony kernel - self::bootKernel(); + $clock = new MockClock('2024-01-01'); + $daysAgos = new \DateTimeImmutable('2023-09-01'); - // Get the actual services from the container - $this->em = self::getContainer()->get(EntityManagerInterface::class); - $this->registry = self::getContainer()->get(Registry::class); - $this->logger = self::getContainer()->get(LoggerInterface::class); - $this->clock = self::getContainer()->get(ClockInterface::class); - $this->workflowRepository = $this->createMock(EntityWorkflowRepository::class); - } - - public function testWorkflowWithOneStepOlderThan90DaysIsDeleted(): void - { $workflow = new EntityWorkflow(); - $initialStep = new EntityWorkflowStep(); - $initialStep->setTransitionAt(new DateTimeImmutable('-93 days')); - $workflow->addStep($initialStep); + $workflow->setWorkflowName('dummy_workflow'); + $workflow->setCreatedAt(new \DateTimeImmutable('2023-09-01')); + $workflow->setStep('step1', new WorkflowTransitionContextDTO($workflow), 'to_step1', $daysAgos, new User()); - $this->em->persist($workflow); - $this->em->flush(); + $em = $this->prophesize(EntityManagerInterface::class); + $em->flush()->shouldBeCalled(); + $em->remove($workflow)->shouldNotBeCalled(); - $this->handleStaleWorkflow($workflow); + $handler = $this->buildHandler($workflow, $em->reveal(), $clock); - $deletedWorkflow = $this->workflowRepository->find($workflow->getId()); - $this->assertNull($deletedWorkflow, 'The workflow should be deleted.'); + $handler(new CancelStaleWorkflowMessage(1)); - $this->assertNull($this->em->getRepository(EntityWorkflowStep::class)->find($initialStep->getId()), 'The workflow step should be deleted.'); + self::assertEquals('canceled', $workflow->getStep()); + self::assertCount(3, $workflow->getSteps()); } - public function testWorkflowWithMultipleStepsAndNoRecentTransitionsIsCanceled(): void + public function testWorkflowNotInStaledHandlerIsUnrecoverable(): void { + $this->expectException(UnrecoverableMessageHandlingException::class); + + $clock = new MockClock('2024-01-01'); + $daysAgos = new \DateTimeImmutable('2023-12-31'); + $workflow = new EntityWorkflow(); - $step1 = new EntityWorkflowStep(); - $step2 = new EntityWorkflowStep(); + $workflow->setWorkflowName('dummy_workflow'); + $workflow->setCreatedAt(new \DateTimeImmutable('2023-12-31')); + $workflow->setStep('step1', new WorkflowTransitionContextDTO($workflow), 'to_step1', $daysAgos, new User()); - $step1->setTransitionAt(new DateTimeImmutable('-92 days')); - $step2->setTransitionAt(new DateTimeImmutable('-91 days')); + $em = $this->prophesize(EntityManagerInterface::class); + $em->flush()->shouldNotBeCalled(); + $em->remove($workflow)->shouldNotBeCalled(); - $workflow->addStep($step1); - $workflow->addStep($step2); + $handler = $this->buildHandler($workflow, $em->reveal(), $clock); - $this->em->persist($workflow); - $this->em->flush(); - - /** @var WorkflowInterface $workflowComponent */ - $workflowComponent = $this->registry->get($workflowComponent); - - $transitions = $workflowComponent->getEnabledTransitions($workflow); - $metadataStore = $workflowComponent->getMetadataStore(); - - $expectedTransition = null; - - // Find the transition that was expected to be applied by the handler - foreach ($transitions as $transition) { - $isFinal = $metadataStore->getMetadata('isFinal', $transition); - $isFinalPositive = $metadataStore->getMetadata('isFinalPositive', $transition); - - if ($isFinal === true && $isFinalPositive === false) { - $expectedTransition = $transition; - break; - } - } - - $this->assertNotNull($expectedTransition, 'Expected to find a valid transition with isFinal = true and isFinalPositive = false.'); - - $this->handleStaleWorkflow($workflow); - $updatedWorkflow = $this->workflowRepository->find($workflow->getId()); - - $this->assertEquals($expectedTransition->getName(), $updatedWorkflow->getCurrentStep()); + $handler(new CancelStaleWorkflowMessage(1)); + self::assertEquals('canceled', $workflow->getStep()); + self::assertCount(3, $workflow->getSteps()); } - public function handleStaleWorkflow($workflow): void + public function testWorkflowStaledInInitialStateIsCompletelyRemoved(): void { - $handler = new CancelStaleWorkflowHandler($this->workflowRepository, $this->registry, $this->em, $this->logger, $this->clock); - $handler(new CancelStaleWorkflowMessage($workflow->getId())); + $clock = new MockClock('2024-01-01'); + + $workflow = new EntityWorkflow(); + $workflow->setWorkflowName('dummy_workflow'); + $workflow->setCreatedAt(new \DateTimeImmutable('2023-09-01')); + + $em = $this->prophesize(EntityManagerInterface::class); + $em->flush()->shouldBeCalled(); + $em->remove($workflow)->shouldBeCalled(); + + $handler = $this->buildHandler($workflow, $em->reveal(), $clock); + + $handler(new CancelStaleWorkflowMessage(1)); + + self::assertEquals('canceled', $workflow->getStep()); + self::assertCount(2, $workflow->getSteps()); } + private function buildHandler( + EntityWorkflow $entityWorkflow, + EntityManagerInterface $entityManager, + ClockInterface $clock, + ): CancelStaleWorkflowHandler { + // set an id for the workflow + $reflection = new \ReflectionClass($entityWorkflow); + $reflection->getProperty('id')->setValue($entityWorkflow, 1); + + $repository = $this->prophesize(EntityWorkflowRepository::class); + $repository->find(1)->willReturn($entityWorkflow); + + return new CancelStaleWorkflowHandler($repository->reveal(), $this->buildRegistry(), $entityManager, new NullLogger(), $clock); + } + + private function buildRegistry(): Registry + { + $definitionBuilder = new DefinitionBuilder(); + + $definitionBuilder + ->setInitialPlaces('initial') + ->addPlaces(['initial', 'step1', 'canceled', 'final']) + ->addTransition(new Transition('to_step1', 'initial', 'step1')) + ->addTransition($cancelInit = new Transition('cancel', 'initial', 'canceled')) + ->addTransition($finalizeInit = new Transition('finalize', 'initial', 'final')) + ->addTransition($cancelStep1 = new Transition('cancel', 'step1', 'canceled')) + ->addTransition($finalizeStep1 = new Transition('finalize', 'step1', 'final')); + + $transitionStorage = new \SplObjectStorage(); + $transitionStorage->attach($finalizeInit, ['isFinal' => true, 'isFinalPositive' => true]); + $transitionStorage->attach($cancelInit, ['isFinal' => true, 'isFinalPositive' => false]); + $transitionStorage->attach($finalizeStep1, ['isFinal' => true, 'isFinalPositive' => true]); + $transitionStorage->attach($cancelStep1, ['isFinal' => true, 'isFinalPositive' => false]); + + $definitionBuilder->setMetadataStore(new InMemoryMetadataStore([], [], $transitionStorage)); + $workflow = new Workflow($definitionBuilder->build(), new EntityWorkflowMarkingStore(), null, 'dummy_workflow'); + $supports = + new class () implements WorkflowSupportStrategyInterface { + public function supports(WorkflowInterface $workflow, object $subject): bool + { + return true; + } + }; + + + $registry = new Registry(); + $registry->addWorkflow($workflow, $supports); + + return $registry; + } } From 2fb46c65c27893e867749ad27462863b9290349e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 9 Sep 2024 15:16:10 +0200 Subject: [PATCH 46/60] Refactor CancelStaleWorkflowCronJobTest to simplify setup Replaced KernelTestCase with TestCase to simplify test setup and removed dependency on the database connection. Added NullLogger to replace mocked LoggerInterface during testing. Updated method call in tests to correctly reference CancelStaleWorkflowMessage class. --- .../Workflow/CancelStaleWorkflowCronJob.php | 2 +- .../CancelStaleWorkflowCronJobTest.php | 18 +++++------------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php index 767af2c5c..be3dd2a5e 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php @@ -49,7 +49,7 @@ class CancelStaleWorkflowCronJob implements CronJobInterface $olderThanDate = $this->clock->now()->sub(new \DateInterval(self::KEEP_INTERVAL)); $staleWorkflowIds = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate); - $lastCanceled = self::LAST_CANCELED_WORKFLOW; + $lastCanceled = $lastExecutionData[self::LAST_CANCELED_WORKFLOW] ?? 0; $processedCount = 0; foreach ($staleWorkflowIds as $wId) { diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php index 07bca6bc5..200b92099 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php @@ -13,13 +13,12 @@ namespace Services\Workflow; use Chill\MainBundle\Entity\CronJobExecution; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; -use Chill\MainBundle\Service\Workflow\CancelStaleWorkflow; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowCronJob; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowMessage; -use Doctrine\DBAL\Connection; use PHPUnit\Framework\MockObject\Exception; +use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use Psr\Log\NullLogger; use Symfony\Component\Clock\MockClock; use Symfony\Component\Messenger\Envelope; use Symfony\Component\Messenger\MessageBusInterface; @@ -29,14 +28,8 @@ use Symfony\Component\Messenger\MessageBusInterface; * * @coversNothing */ -class CancelStaleWorkflowCronJobTest extends KernelTestCase +class CancelStaleWorkflowCronJobTest extends TestCase { - protected function setUp(): void - { - self::bootKernel(); - $this->connection = self::getContainer()->get(Connection::class); - } - /** * @dataProvider buildTestCanRunData * @@ -61,12 +54,11 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase { $clock = new MockClock((new \DateTimeImmutable('now', new \DateTimeZone('+00:00')))->add(new \DateInterval('P120D'))); $workflowRepository = $this->createMock(EntityWorkflowRepository::class); - $logger = $this->createMock(LoggerInterface::class); $workflowRepository->method('findWorkflowsWithoutFinalStepAndOlderThan')->willReturn([1, 3, 2]); $messageBus = $this->buildMessageBus(true); - $cronJob = new CancelStaleWorkflowCronJob($workflowRepository, $clock, $messageBus, $logger); + $cronJob = new CancelStaleWorkflowCronJob($workflowRepository, $clock, $messageBus, new NullLogger()); $results = $cronJob->run([]); @@ -101,7 +93,7 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase $messageBus = $this->createMock(MessageBusInterface::class); $methodDispatch = match ($expectDispatchAtLeastOnce) { - true => $messageBus->expects($this->atLeastOnce())->method('dispatch')->with($this->isInstanceOf(CancelStaleWorkflow::class)), + true => $messageBus->expects($this->atLeastOnce())->method('dispatch')->with($this->isInstanceOf(CancelStaleWorkflowMessage::class)), false => $messageBus->method('dispatch'), }; From f5ba5d574b905ae66755d5c6f7dc733111cb5e22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Tue, 10 Sep 2024 10:44:45 +0200 Subject: [PATCH 47/60] Add WopiConverter service and update Collabora integration tests Introduce the WopiConverter service to handle document-to-PDF conversion using Collabora Online. Extend and update related tests in WopiConvertToPdfTest and ConvertControllerTest for better coverage and reliability. Enhance the GitLab CI configuration to exclude new test category "collabora-integration". --- .env | 2 +- .env.test | 2 + .gitlab-ci.yml | 2 +- .../src/Controller/ConvertController.php | 64 +++------------- .../src/Service/WopiConverter.php | 69 ++++++++++++++++++ .../Controller/ConvertControllerTest.php | 56 ++++++-------- .../tests/Service/WopiConvertToPdfTest.php | 63 ++++++++++++++++ .../tests/Service/fixtures/test-document.odt | Bin 0 -> 8918 bytes tests/app/config/packages/wopi.yaml | 2 + 9 files changed, 171 insertions(+), 89 deletions(-) create mode 100644 src/Bundle/ChillWopiBundle/src/Service/WopiConverter.php create mode 100644 src/Bundle/ChillWopiBundle/tests/Service/WopiConvertToPdfTest.php create mode 100644 src/Bundle/ChillWopiBundle/tests/Service/fixtures/test-document.odt create mode 100644 tests/app/config/packages/wopi.yaml diff --git a/.env b/.env index 1714966d4..b2eecb78f 100644 --- a/.env +++ b/.env @@ -23,7 +23,7 @@ TRUSTED_HOSTS='^(localhost|example\.com|nginx)$' ###< symfony/framework-bundle ### ## Wopi server for editing documents online -WOPI_SERVER=http://collabora:9980 +EDITOR_SERVER=http://collabora:9980 # must be manually set in .env.local # ADMIN_PASSWORD= diff --git a/.env.test b/.env.test index f84920e54..c78a1bc63 100644 --- a/.env.test +++ b/.env.test @@ -41,3 +41,5 @@ DATABASE_URL="postgresql://postgres:postgres@db:5432/test?serverVersion=14&chars ASYNC_UPLOAD_TEMP_URL_KEY= ASYNC_UPLOAD_TEMP_URL_BASE_PATH= ASYNC_UPLOAD_TEMP_URL_CONTAINER= + +EDITOR_SERVER=https://localhost:9980 diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index acd66a42e..c1fdebf43 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -122,7 +122,7 @@ unit_tests: - php tests/console chill:db:sync-views --env=test - php -d memory_limit=2G tests/console cache:clear --env=test - php -d memory_limit=3G tests/console doctrine:fixtures:load -n --env=test - - php -d memory_limit=4G bin/phpunit --colors=never --exclude-group dbIntensive,openstack-integration + - php -d memory_limit=4G bin/phpunit --colors=never --exclude-group dbIntensive,openstack-integration,collabora-integration artifacts: expire_in: 1 day paths: diff --git a/src/Bundle/ChillWopiBundle/src/Controller/ConvertController.php b/src/Bundle/ChillWopiBundle/src/Controller/ConvertController.php index 73207e13a..86a624e4b 100644 --- a/src/Bundle/ChillWopiBundle/src/Controller/ConvertController.php +++ b/src/Bundle/ChillWopiBundle/src/Controller/ConvertController.php @@ -14,84 +14,44 @@ namespace Chill\WopiBundle\Controller; use Chill\DocStoreBundle\Entity\StoredObject; use Chill\DocStoreBundle\Service\StoredObjectManager; use Chill\DocStoreBundle\Service\StoredObjectManagerInterface; -use Chill\MainBundle\Entity\User; +use Chill\WopiBundle\Service\WopiConverter; use Psr\Log\LoggerInterface; -use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface; -use Symfony\Component\HttpFoundation\JsonResponse; -use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; -use Symfony\Component\Mime\Part\DataPart; -use Symfony\Component\Mime\Part\Multipart\FormDataPart; use Symfony\Component\Security\Core\Security; -use Symfony\Contracts\HttpClient\Exception\ClientExceptionInterface; -use Symfony\Contracts\HttpClient\Exception\RedirectionExceptionInterface; -use Symfony\Contracts\HttpClient\Exception\ServerExceptionInterface; -use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface; -use Symfony\Contracts\HttpClient\HttpClientInterface; -use Symfony\Contracts\HttpClient\ResponseInterface; class ConvertController { private const LOG_PREFIX = '[convert] '; - private readonly string $collaboraDomain; - /** * @param StoredObjectManager $storedObjectManager */ public function __construct( - private readonly HttpClientInterface $httpClient, - private readonly RequestStack $requestStack, private readonly Security $security, private readonly StoredObjectManagerInterface $storedObjectManager, + private readonly WopiConverter $wopiConverter, private readonly LoggerInterface $logger, - ParameterBagInterface $parameters, - ) { - $this->collaboraDomain = $parameters->get('wopi')['server']; - } + ) {} - public function __invoke(StoredObject $storedObject): Response + public function __invoke(StoredObject $storedObject, Request $request): Response { - if (!$this->security->getUser() instanceof User) { + if (!($this->security->isGranted('ROLE_USER') || $this->security->isGranted('ROLE_ADMIN'))) { throw new AccessDeniedHttpException('User must be authenticated'); } $content = $this->storedObjectManager->read($storedObject); - $query = []; - if (null !== $request = $this->requestStack->getCurrentRequest()) { - $query['lang'] = $request->getLocale(); - } + $lang = $request->getLocale(); try { - $url = sprintf('%s/cool/convert-to/pdf', $this->collaboraDomain); - $form = new FormDataPart([ - 'data' => new DataPart($content, $storedObject->getUuid()->toString(), $storedObject->getType()), - ]); - $response = $this->httpClient->request('POST', $url, [ - 'headers' => $form->getPreparedHeaders()->toArray(), - 'query' => $query, - 'body' => $form->bodyToString(), - 'timeout' => 10, - ]); - - return new Response($response->getContent(), Response::HTTP_OK, [ + return new Response($this->wopiConverter->convert($lang, $content, $storedObject->getType()), Response::HTTP_OK, [ 'Content-Type' => 'application/pdf', ]); - } catch (ClientExceptionInterface|RedirectionExceptionInterface|ServerExceptionInterface|TransportExceptionInterface $exception) { - return $this->onConversionFailed($url, $exception->getResponse()); + } catch (\RuntimeException $exception) { + $this->logger->alert(self::LOG_PREFIX.'Could not convert document', ['message' => $exception->getMessage(), 'exception', $exception->getTraceAsString()]); + + return new Response('convert server not available', Response::HTTP_SERVICE_UNAVAILABLE); } } - - private function onConversionFailed(string $url, ResponseInterface $response): JsonResponse - { - $this->logger->error(self::LOG_PREFIX.' could not convert document', [ - 'response_status' => $response->getStatusCode(), - 'message' => $response->getContent(false), - 'server' => $this->collaboraDomain, - 'url' => $url, - ]); - - return new JsonResponse(['message' => 'conversion failed : '.$response->getContent(false)], Response::HTTP_SERVICE_UNAVAILABLE); - } } diff --git a/src/Bundle/ChillWopiBundle/src/Service/WopiConverter.php b/src/Bundle/ChillWopiBundle/src/Service/WopiConverter.php new file mode 100644 index 000000000..ded9b3188 --- /dev/null +++ b/src/Bundle/ChillWopiBundle/src/Service/WopiConverter.php @@ -0,0 +1,69 @@ +collaboraDomain = $parameters->get('wopi')['server']; + } + + public function convert(string $lang, string $content, string $contentType, $convertTo = 'pdf'): string + { + try { + $url = sprintf('%s/cool/convert-to/%s', $this->collaboraDomain, $convertTo); + + $form = new FormDataPart([ + 'data' => new DataPart($content, uniqid('temp-file-'), contentType: $contentType), + ]); + $response = $this->httpClient->request('POST', $url, [ + 'headers' => $form->getPreparedHeaders()->toArray(), + 'query' => ['lang' => $lang], + 'body' => $form->bodyToString(), + 'timeout' => 10, + ]); + + if (200 === $response->getStatusCode()) { + $this->logger->info(self::LOG_PREFIX.'document converted successfully', ['size' => strlen($content)]); + } + + return $response->getContent(); + } catch (ClientExceptionInterface $e) { + throw new \LogicException('no correct request to collabora online', previous: $e); + } catch (RedirectionExceptionInterface $e) { + throw new \RuntimeException('no redirection expected', previous: $e); + } catch (ServerExceptionInterface|TransportExceptionInterface $e) { + throw new \RuntimeException('error while converting document', previous: $e); + } + } +} diff --git a/src/Bundle/ChillWopiBundle/tests/Controller/ConvertControllerTest.php b/src/Bundle/ChillWopiBundle/tests/Controller/ConvertControllerTest.php index 0a928bcd9..9a529df6b 100644 --- a/src/Bundle/ChillWopiBundle/tests/Controller/ConvertControllerTest.php +++ b/src/Bundle/ChillWopiBundle/tests/Controller/ConvertControllerTest.php @@ -13,16 +13,12 @@ namespace Chill\WopiBundle\Tests\Controller; use Chill\DocStoreBundle\Entity\StoredObject; use Chill\DocStoreBundle\Service\StoredObjectManagerInterface; -use Chill\MainBundle\Entity\User; use Chill\WopiBundle\Controller\ConvertController; +use Chill\WopiBundle\Service\WopiConverter; use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; use Psr\Log\NullLogger; -use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag; -use Symfony\Component\HttpClient\MockHttpClient; -use Symfony\Component\HttpClient\Response\MockResponse; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\Security\Core\Security; /** @@ -39,28 +35,27 @@ final class ConvertControllerTest extends TestCase $storedObject = new StoredObject(); $storedObject->registerVersion(type: 'application/vnd.oasis.opendocument.text'); - $httpClient = new MockHttpClient([ - new MockResponse('not authorized', ['http_code' => 401]), - ], 'http://collabora:9980'); - $security = $this->prophesize(Security::class); - $security->getUser()->willReturn(new User()); + $security->isGranted('ROLE_USER')->willReturn(true); $storeManager = $this->prophesize(StoredObjectManagerInterface::class); $storeManager->read($storedObject)->willReturn('content'); - $parameterBag = new ParameterBag(['wopi' => ['server' => 'http://collabora:9980']]); + $wopiConverter = $this->prophesize(WopiConverter::class); + $wopiConverter->convert('fr', 'content', 'application/vnd.oasis.opendocument.text') + ->willThrow(new \RuntimeException()); - $convert = new ConvertController( - $httpClient, - $this->makeRequestStack(), + $controller = new ConvertController( $security->reveal(), $storeManager->reveal(), + $wopiConverter->reveal(), new NullLogger(), - $parameterBag ); - $response = $convert($storedObject); + $request = new Request(); + $request->setLocale('fr'); + + $response = $controller($storedObject, $request); $this->assertNotEquals(200, $response->getStatusCode()); } @@ -70,38 +65,29 @@ final class ConvertControllerTest extends TestCase $storedObject = new StoredObject(); $storedObject->registerVersion(type: 'application/vnd.oasis.opendocument.text'); - $httpClient = new MockHttpClient([ - new MockResponse('1234', ['http_code' => 200]), - ], 'http://collabora:9980'); - $security = $this->prophesize(Security::class); - $security->getUser()->willReturn(new User()); + $security->isGranted('ROLE_USER')->willReturn(true); $storeManager = $this->prophesize(StoredObjectManagerInterface::class); $storeManager->read($storedObject)->willReturn('content'); - $parameterBag = new ParameterBag(['wopi' => ['server' => 'http://collabora:9980']]); + $wopiConverter = $this->prophesize(WopiConverter::class); + $wopiConverter->convert('fr', 'content', 'application/vnd.oasis.opendocument.text') + ->willReturn('1234'); - $convert = new ConvertController( - $httpClient, - $this->makeRequestStack(), + $controller = new ConvertController( $security->reveal(), $storeManager->reveal(), + $wopiConverter->reveal(), new NullLogger(), - $parameterBag ); - $response = $convert($storedObject); + $request = new Request(); + $request->setLocale('fr'); + + $response = $controller($storedObject, $request); $this->assertEquals(200, $response->getStatusCode()); $this->assertEquals('1234', $response->getContent()); } - - private function makeRequestStack(): RequestStack - { - $requestStack = new RequestStack(); - $requestStack->push(new Request()); - - return $requestStack; - } } diff --git a/src/Bundle/ChillWopiBundle/tests/Service/WopiConvertToPdfTest.php b/src/Bundle/ChillWopiBundle/tests/Service/WopiConvertToPdfTest.php new file mode 100644 index 000000000..317932ea5 --- /dev/null +++ b/src/Bundle/ChillWopiBundle/tests/Service/WopiConvertToPdfTest.php @@ -0,0 +1,63 @@ + ['server' => $_ENV['EDITOR_SERVER']], + ]); + + $converter = new WopiConverter($client, new NullLogger(), $parameters); + + $actual = $converter->convert('fr', $content, 'application/vnd.oasis.opendocument.text'); + + self::assertIsString($actual); + } + + public function testConvertToPdfWithMock(): void + { + $httpClient = new MockHttpClient([ + new MockResponse('1234', ['http_code' => 200]), + ], 'http://collabora:9980'); + $parameters = new ParameterBag([ + 'wopi' => ['server' => 'http://collabora:9980'], + ]); + + $converter = new WopiConverter($httpClient, new NullLogger(), $parameters); + + $actual = $converter->convert('fr', 'content-string', 'application/vnd.oasis.opendocument.text'); + + self::assertEquals('1234', $actual); + } +} diff --git a/src/Bundle/ChillWopiBundle/tests/Service/fixtures/test-document.odt b/src/Bundle/ChillWopiBundle/tests/Service/fixtures/test-document.odt new file mode 100644 index 0000000000000000000000000000000000000000..b6f6444082151390d1fc8e52ea568c70c745d22b GIT binary patch literal 8918 zcmb7K1z40@w;n)3S|p@PQeprJNhy^^x_gMB24;u>0cnsFq@*My3`#^gq+1$E>1JpU zi94Kg{nc~P^WViY4{PT6-gmFPcFf-E9aROiE5rZ*768x^5T>|P#Y+hBLw7N;b`t+4|0HU!9eaX)nCYv`2QFc z(vx;{u(YvqasG=2%FAtPZVs{oA>TSWbN?PmdkNCU-qZ>N<(9I6*_%S3e*jZl0){y{ z+CfYmKz4t?T^26H+0n`w1cjQJ{wdtcT7<$(VJ`oIyYZVy|J>STC`&s>QyA#~8zaOK z;sQYqq{|<&a#?E^kRLH(VPX9>kC2`DkKrOcds7D+OAr*sE^(G$2dNaQxMm#_gyBd`}185D zAt9#+xGklTSXXHxq-^%*A5q-tnZ0VM{SdbHA{u^3Cl)p-od2Q(+q6urCb*35sqfv0 ztBM@~Fn_8_Wrb$G_}zzh8ZtI!TTN;P{4+&Owr)lCj!uQ(<}4U}sVUkt@SlDZXYH|R z?%^`fygkkM%p`?(Q)RVE1b)7B#?^aHK3iDoDr0xIC(G(MdZ~1WR`Y(Lssbif>7jK$ zItl>bj|KqzJ^wFD-(SlR1T*Dwx3`N>SBgpo5?3E;(^YB%TRszVn7kC*D^D+&B>ZYcksqIkN)p{aPgbbOdM52w{?w*@ z(7KlZ zCvuPH0CQr>ta8idL^(oCa+S>wgEm=t8&j zl#3DHuob;hjU4gzKaTjtf`NiyFdGLe=*6hl=ubl_?%;aY)iE@B%3F_>n#RB$2L#=^ zg7JWFMDX3Mda}YAb5FFBLl!u_5?b~%*~2UA1nu!JRnje>+n}_(ES4f=3NPJP)go-v zzLAO?`|~9*2|F4eF0YE?wU6PgynbN2Ob(QdC~EhpsB*?fhE-sTJ)K#cq#;el6qqI_ z5k!+#QwiH+UvGb5VCV%$9j~^Ugi(CXRPK;7Tp+n7IklG0RjnW;Jw2guaww}MaS2n- zRlHsxFr!NSSm?!e4Q0h)dfFRtC!(?A<#1?^iRZ*qKGic=KMoH)d8S|n#JfYe-d zKhkTb-|R|zt2MSaRfu@hx@1e*^eOx05Ov*y?MPk^wMyX%*21t+Lp#crH0e$M@KadU zmw{fZIKPjMlFB*B!G?Dvz8-l?toY|A?bF|*^>5KwzFMwLaj%7yuoAXlkMV8AdZm(| z50mn;!c~UrmOEM};JhAvxfW*Qse#d(Zm$uV z?72&Erwa}^afh@j2l1=(4;p6tnL@GeGaGLvWK zjZ6vf50jy0m+2&S35ItNoeJkA^y8H4bJaL>%{63tOmZn*gPVUq_CjU6bOJyuLo=Jx zjDstEjuCwjm=GlCs&rq-CnOknXsFn04kA=zX3$&Duxo5i09I@q5Wxi{9_xC$8AThn zagZyVUzJ>@C;gVj&($1f0fa_RW6nx!316d9JOi-2{^H0t_8b}prd!{<8lO!wPk=tl zkgydbDR}#dg6QX$T$!z*{7F-lE$DFu`aC#X-Hf#b5LBho>&ciyr2^ssOBgBK@!8G< zQdDa*94255+`{5B-~fp0=u0Qot_8ZQ zzLNsti+!MrM(1#<#MhX>TAG%3lLfN{?x5jF;!hC@qq-d3Z8%zfC1i~JJ? z3LJuLQ>?_gt#32h*-v%D6udYI%>o*`(r-$&Qc`FM&D7jNxoH~p)uN1y-1FcS*hz7@ z*kCo|Dv2!`@qJzLW4ZTT$>bYoq7NiC=-z@*3%haNRahBL^_e%QnGEz0%J+yo(trL% z{Z0~%xs)`+1AZx#_;7kh8zMy#XZ0?`aMh!#@rAgg6{ra>B}sJ1vlojL>XHit{rjNp zU6bvHVd$z`43Wfr&eQd)^>;m!?nx1KctnmVn?xougxJ!4GZMR^f2`iUOT;tmbllk`x(4|W)L)gulZM?K-{VOvExi#+8F}WQ)E;_|o5DS|zypMPVjIVjC+mAP zA<~@cQ5A&TFpw(r?)ht8#*Bi5R{4;j;{BHmI|FN)Y4ASbc{#p-``pjAN(Pr~T<)Js z&BMp5P2bk4UEhl+`Eb`#+Vx(Q;4Nn0gMhIPp(p&AgzY}Okp=hWG8S%VxJ;mt<`Qp6 zhwX)q(aCRJA@b1Ww7a&rAnMT!?Hqd#(-*Ey)fCQYDCMbG573B!QrrZ?U!M zDJ>Jtcs~LDXnl%d`|Z@{fqy}F>b8$nPv^E@&WuzIJB*oFTkt$V>DyUR_e%!51Rl6H zP@k46AjhKHa}DSK*~s_rilvs49HowW$fBToVuZ2kTC{gA*UzToOndYfRUf1W?3DS~8k}NYe6K3cCnwTi0ssh} z?;)r$gu?$WvcvN~BL} z`pajTMR}#wC8ecrYkP|8dW-9OOF#D2SC=<7)Hn7`PL1{s4h#$qjSh}ZjC~#$nwlA# zoSFDCH?p`pwzN0Bc{IPWxx9v$MV#zxuI+8D@9ZBP?QNZ%ogv#lKR;(lh591HLdeNT zYPgSW4BKM+-30{pykVV)PO- zTQ2wvHkloyZMr%VU1hU5wOikbCiy373Vzg!2;Qq^{aYx=;J$});aotF!TpT=FG#ZP zJ-!5ZS^qCh#4p%?w0;@MAl6?&UK;H$&;Kz+fA{p?Ng^mv{w+oS+vtBe{)aM1ki*V%-xk9c1F5%h@mjJ z9q6yq^M;;r)Oh(dA4Dz?R$kjzvOd{4*1h;Rhtp%^CZ<9E5*p`2Qc8K^2jH5XT*Owo zOnZ#>=WY_tRWF^pDKlM=MZre;l@ST?c)RVp0om1+Q85ebdeQ6)8Mq_hRDq;zZ7}fTIfawM1;q#KYk?Cm5TF*bpaGXGRa4T-F`*9CoU6C ze|rYDkykOm?J7FBhr&Rb>8WBB+kL1w0+^0s5%>eIf} z-5X^w8?Ka;udKSi8Zbr0WTdDBZ1EmAVvK$;hLThbuAx}Qc4KxiuVtVkmN5H(!S7z@ zOkC&D7t}hU!u)y?*jVk7ocFj1e`WU5aAt}iiM8MHLiW?v=UA`g5|eDI(gOG#MK#ND z)f3;4vD|#Ev+I85-^MEtBuiKmn!)88^$Crs6>);wl3uKc8FN>P09c&@q!0B)dx~jD zHlFe#0RHIxYf8Z6q+>jC1?ug|*m}e0NCGhBD@E1VJgN9K*_vihaKKws zy@Gf$i|_0P!ui6lS2qoXh0mdDhSf!58qA<|B?&mxFNc4V-)v#tkWIhQpB z+~_a@%<3+~iY-=Tl&az9!~BT~h+j45mYhuMu&q@3i4uiarbVyg^}c`#eUfvnh#zZ{ zEeQOe`RSRfx`i*Fw~Wc#o7iGg_g^nCbKVclMR_P0{NzO={exklK&@*e2A{YM$n3f+ zYpdE$UXaD5y_4EVxwRkeB9jB<^>?AlXOG@6g?3pyE~LP*JDJ=qqqp%6plp7R51{VG z`tT-k2>k#%oZM3q2=gaoc+RfU*PT=LcAi|(3N_!0K%=xHWD2;a%VH2N*guX{gYqS4 zL@mo(hIvqu|dT=F!Dzp?V$L_cOUlRDri(2}2{qN9l^bgaJ+34fMR|Y&>tHYnivVM1vhC z+sxX`7^$35-l!XNny9-wV2sV_s}_-8-L~p2_vC^p+thy)M_)F7VPMs$K*HD%g|-0X zeQ??vF!*2_?O~v};o7>Fyi?vM%li9m&A4G4&)?h?uf|n?yisoA;_O{GP5r`=hZ#F- z@}lON%v_F!@RD?p3UF)OV}Cr&DwpS7+N(GwQq|Y>p!W{!4#PS5%PofnY+mz$-i*3# zAhy6~fE7H;#*Jz`zm!O)OtEW}dT3F5X_c^$FpVNJ(0i%+<%Ts);y)u#!~r~1XNxb1=+j+4Bz==N0aZZfrR9PZZa;UkM?9OqIbW_t7CU5E^qQ%!3x zaUCm>MNWa%PDd=ft$=!C-84fyZ#lRCg7jeLGgYfax>?a2B1e0G$upCw)Z7E3v=fpU zxO=vKLqGk_YTNuY4%%*Ib$wI(W_Su-X8IT6lOsHOnTYBz!sB~0XN8^W=3W`O_VUWH%`bN>(XQ#`b8`n zg)LZqIx#Agx=84=B`_G{IjvmK0gJvu#xC9j513LlH(qRje(kmNG5%)*>VCJj?jd@w z;0dD(mq0ssk9ucyHM&-MvjBfZ#KP01?FEniDJ?u(1$$D%N%m%Gho&+?MAbICI%;kQK-cLdC1kDPXG^pmHM^WQH3dL))PkSF zMrc+zZm9FYn~Dpo7M-D08zFyP|0Jm1#x8DQzY_%WEMRNw;Ow zz7SYph6spD!aOnK4qcLD7#02Bm1R)b@663y)U+t z^_Z71 zi4=%BQ+^G@+RGv5xCxilQ#8u4>sz5SBcko>PS9o{DV^aDKWFp`B@T=)Q z(it3eLRs0Ea~-cO`#KGL(`8It>mEg4Usy(`Bk?vdN(d$ibmz)zT)iViozYYh*9V6B z>-9H}JpHr?7?g~}ZEBs9@b%txytqVAzNp$@LRMR)IKN*aE-T04AP*Pjjt;O3$>X9n zr>*ZeEM`EFMp%u<4sT+^dm33oMA}^ zvy2w$*}WHrNpIrh*|fKZ_2TGnlDr<9yp@jwgL>7-YzS|{LU?dQz9i>VfJte-oe(&e zG?moIidBAk_GILf3EUT3rntF1^4+3RVe$0Xj%Ea`!hnH$Bcoz)!JA~HvLn*Tii9hv zebk7j+ptH+Ld{Q#!6pszz*r%U8qeZvU3faEx9NFiZxn%wMW;NjKzQ@ewoGY>RS$x* zUs2B9^EMNj&XAt_4W^K*i+=p9HS)m&jQVfv6X*mcU=_NvyBlH8BkJeux(cv{&{z*ZFHH6&}S?*}JZ1SK_D^iYUah zBA^gl(+%9CxT6?Fwj<)VDKB0EWUJ$_x}Qh+(QYYcq8`$71uyvvFI)G@k?YvNSEV}f zx?tuOSCN5Wx8-XAXG`0rC-1LXDRU(R463)VcjrPW_8 z((pdn={C>CmRtinD>ttD0K}XbtC`ccf4yiiZIV^=HZTdZdGRqygN^R&YUY>u`0-`N z@fmEg7kso6SBr2qDZ6v0!RK}${FaiPnDz4*Ee{$Q+Os1*k7FR^B-<9ty`@A#(kZO3 zjkD6w?$M38F;sL;me3D;u>A8>i(NP2qg69)ojED<_Z#R%Gc+jPPfPVwAaO5s2FeB3 z9ZW<0=sQanx?~zD>HRZM=SVD9m<`WdDWh)sXU$)kBcf{dR6GRfrJ69VnCg`43?H5u zS&yFci$a3t;1%RwIXfbVj-uPfK59N;LKPRxC=*T&7v9+QxxjX6X~@U%r9qB~#4%0ogYMn8u zx}m9t$S~{hv|a6s7BX*aCEo(ArIg??54#`1@|2Q4hD12dN^T@ZFAc+atBr_;Lsi^F zZy%$~80^Q$YJ&m!4Xe~=K%Qa>3SU-){UhNcclk*IcJr(kD{W$BZZe9wdW6#6sa~s` zqSKG2u27V1@oXxed|Ulye6fi>QwYZ4L@FFiS1$66f=Uedy||1_`cL* Date: Tue, 10 Sep 2024 12:05:23 +0200 Subject: [PATCH 48/60] Add StoredObjectPointInTime entity and related functionality Implemented a new StoredObjectPointInTime entity to manage snapshots of stored objects. This includes related migrations, enum for reasons, repository, and integration with StoredObjectVersion. --- .../Entity/StoredObjectPointInTime.php | 67 +++++++++++++++++++ .../StoredObjectPointInTimeReasonEnum.php | 18 +++++ .../Entity/StoredObjectVersion.php | 46 +++++++++++++ .../StoredObjectPointInTimeRepository.php | 27 ++++++++ .../migrations/Version20240910093735.php | 49 ++++++++++++++ 5 files changed, 207 insertions(+) create mode 100644 src/Bundle/ChillDocStoreBundle/Entity/StoredObjectPointInTime.php create mode 100644 src/Bundle/ChillDocStoreBundle/Entity/StoredObjectPointInTimeReasonEnum.php create mode 100644 src/Bundle/ChillDocStoreBundle/Repository/StoredObjectPointInTimeRepository.php create mode 100644 src/Bundle/ChillDocStoreBundle/migrations/Version20240910093735.php diff --git a/src/Bundle/ChillDocStoreBundle/Entity/StoredObjectPointInTime.php b/src/Bundle/ChillDocStoreBundle/Entity/StoredObjectPointInTime.php new file mode 100644 index 000000000..bff5c60c1 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Entity/StoredObjectPointInTime.php @@ -0,0 +1,67 @@ +objectVersion->addPointInTime($this); + } + + public function getId(): ?int + { + return $this->id; + } + + public function getByUser(): ?User + { + return $this->byUser; + } + + public function getObjectVersion(): StoredObjectVersion + { + return $this->objectVersion; + } + + public function getReason(): StoredObjectPointInTimeReasonEnum + { + return $this->reason; + } +} diff --git a/src/Bundle/ChillDocStoreBundle/Entity/StoredObjectPointInTimeReasonEnum.php b/src/Bundle/ChillDocStoreBundle/Entity/StoredObjectPointInTimeReasonEnum.php new file mode 100644 index 000000000..9f03c7279 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Entity/StoredObjectPointInTimeReasonEnum.php @@ -0,0 +1,18 @@ + ''])] private string $filename = ''; + /** + * @var Collection&Selectable + */ + #[ORM\OneToMany(mappedBy: 'objectVersion', targetEntity: StoredObjectPointInTime::class, cascade: ['persist', 'remove'], orphanRemoval: true)] + private Collection&Selectable $pointInTimes; + public function __construct( /** * The stored object associated with this version. @@ -77,6 +86,7 @@ class StoredObjectVersion implements TrackCreationInterface ?string $filename = null, ) { $this->filename = $filename ?? self::generateFilename($this); + $this->pointInTimes = new ArrayCollection(); } public static function generateFilename(StoredObjectVersion $storedObjectVersion): string @@ -124,4 +134,40 @@ class StoredObjectVersion implements TrackCreationInterface { return $this->version; } + + /** + * @return Collection&Selectable + */ + public function getPointInTimes(): Selectable&Collection + { + return $this->pointInTimes; + } + + public function hasPointInTimes(): bool + { + return $this->pointInTimes->count() > 0; + } + + /** + * @return $this + * + * @internal use @see{StoredObjectPointInTime} constructor instead + */ + public function addPointInTime(StoredObjectPointInTime $storedObjectPointInTime): self + { + if (!$this->pointInTimes->contains($storedObjectPointInTime)) { + $this->pointInTimes->add($storedObjectPointInTime); + } + + return $this; + } + + public function removePointInTime(StoredObjectPointInTime $storedObjectPointInTime): self + { + if ($this->pointInTimes->contains($storedObjectPointInTime)) { + $this->pointInTimes->removeElement($storedObjectPointInTime); + } + + return $this; + } } diff --git a/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectPointInTimeRepository.php b/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectPointInTimeRepository.php new file mode 100644 index 000000000..c5c923ac9 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectPointInTimeRepository.php @@ -0,0 +1,27 @@ + + */ +class StoredObjectPointInTimeRepository extends ServiceEntityRepository +{ + public function __construct(ManagerRegistry $registry) + { + parent::__construct($registry, StoredObjectPointInTime::class); + } +} diff --git a/src/Bundle/ChillDocStoreBundle/migrations/Version20240910093735.php b/src/Bundle/ChillDocStoreBundle/migrations/Version20240910093735.php new file mode 100644 index 000000000..21906b168 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/migrations/Version20240910093735.php @@ -0,0 +1,49 @@ +addSql('CREATE SEQUENCE chill_doc.stored_object_point_in_time_id_seq INCREMENT BY 1 MINVALUE 1 START 1'); + $this->addSql('CREATE TABLE chill_doc.stored_object_point_in_time (id INT NOT NULL, stored_object_version_id INT NOT NULL, reason TEXT NOT NULL, createdAt TIMESTAMP(0) WITHOUT TIME ZONE DEFAULT NULL, byUser_id INT DEFAULT NULL, createdBy_id INT DEFAULT NULL, PRIMARY KEY(id))'); + $this->addSql('CREATE INDEX IDX_CC83C7B81D0AB8B9 ON chill_doc.stored_object_point_in_time (stored_object_version_id)'); + $this->addSql('CREATE INDEX IDX_CC83C7B8D23C0240 ON chill_doc.stored_object_point_in_time (byUser_id)'); + $this->addSql('CREATE INDEX IDX_CC83C7B83174800F ON chill_doc.stored_object_point_in_time (createdBy_id)'); + $this->addSql('COMMENT ON COLUMN chill_doc.stored_object_point_in_time.createdAt IS \'(DC2Type:datetime_immutable)\''); + $this->addSql('ALTER TABLE chill_doc.stored_object_point_in_time ADD CONSTRAINT FK_CC83C7B81D0AB8B9 FOREIGN KEY (stored_object_version_id) REFERENCES chill_doc.stored_object_version (id) NOT DEFERRABLE INITIALLY IMMEDIATE'); + $this->addSql('ALTER TABLE chill_doc.stored_object_point_in_time ADD CONSTRAINT FK_CC83C7B8D23C0240 FOREIGN KEY (byUser_id) REFERENCES users (id) NOT DEFERRABLE INITIALLY IMMEDIATE'); + $this->addSql('ALTER TABLE chill_doc.stored_object_point_in_time ADD CONSTRAINT FK_CC83C7B83174800F FOREIGN KEY (createdBy_id) REFERENCES users (id) NOT DEFERRABLE INITIALLY IMMEDIATE'); + $this->addSql('ALTER TABLE chill_doc.stored_object ALTER prefix SET DEFAULT \'\''); + $this->addSql('ALTER TABLE chill_doc.stored_object_version ALTER filename SET DEFAULT \'\''); + } + + public function down(Schema $schema): void + { + $this->addSql('DROP SEQUENCE chill_doc.stored_object_point_in_time_id_seq CASCADE'); + $this->addSql('ALTER TABLE chill_doc.stored_object_point_in_time DROP CONSTRAINT FK_CC83C7B81D0AB8B9'); + $this->addSql('ALTER TABLE chill_doc.stored_object_point_in_time DROP CONSTRAINT FK_CC83C7B8D23C0240'); + $this->addSql('ALTER TABLE chill_doc.stored_object_point_in_time DROP CONSTRAINT FK_CC83C7B83174800F'); + $this->addSql('DROP TABLE chill_doc.stored_object_point_in_time'); + $this->addSql('ALTER TABLE chill_doc.stored_object ALTER prefix DROP DEFAULT'); + $this->addSql('ALTER TABLE chill_doc.stored_object_version ALTER filename DROP DEFAULT'); + } +} From 669b967899527e341a8580164695e66544fd1733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Tue, 10 Sep 2024 12:06:09 +0200 Subject: [PATCH 49/60] Enhance object version removal to exclude point-in-time versions Add a check to exclude versions associated with points in time before deleting old object versions. This ensures that such versions are not mistakenly removed, providing greater data integrity. Updated tests and repository methods accordingly. --- .../Repository/StoredObjectVersionRepository.php | 4 +++- .../StoredObjectCleaner/RemoveOldVersionCronJob.php | 2 +- .../RemoveOldVersionMessageHandler.php | 8 +++++++- .../Repository/StoredObjectVersionRepositoryTest.php | 2 +- .../StoredObjectCleaner/RemoveOldVersionCronJobTest.php | 2 +- 5 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectVersionRepository.php b/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectVersionRepository.php index 1ab9b9edd..60ea07420 100644 --- a/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectVersionRepository.php +++ b/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectVersionRepository.php @@ -62,7 +62,7 @@ class StoredObjectVersionRepository implements ObjectRepository * * @return iterable returns an iterable with the IDs of the versions */ - public function findIdsByVersionsOlderThanDateAndNotLastVersion(\DateTimeImmutable $beforeDate): iterable + public function findIdsByVersionsOlderThanDateAndNotLastVersionAndNotPointInTime(\DateTimeImmutable $beforeDate): iterable { $results = $this->connection->executeQuery( self::QUERY_FIND_IDS_BY_VERSIONS_OLDER_THAN_DATE_AND_NOT_LAST_VERSION, @@ -83,6 +83,8 @@ class StoredObjectVersionRepository implements ObjectRepository sov.createdat < ?::timestamp AND sov.version < (SELECT MAX(sub_sov.version) FROM chill_doc.stored_object_version sub_sov WHERE sub_sov.stored_object_id = sov.stored_object_id) + AND + NOT EXISTS (SELECT 1 FROM chill_doc.stored_object_point_in_time sub_poi WHERE sub_poi.stored_object_version_id = sov.id) SQL; public function getClassName(): string diff --git a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveOldVersionCronJob.php b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveOldVersionCronJob.php index d190a4e45..d2494266b 100644 --- a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveOldVersionCronJob.php +++ b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveOldVersionCronJob.php @@ -50,7 +50,7 @@ final readonly class RemoveOldVersionCronJob implements CronJobInterface $deleteBeforeDate = $this->clock->now()->sub(new \DateInterval(self::KEEP_INTERVAL)); $maxDeleted = $lastExecutionData[self::LAST_DELETED_KEY] ?? 0; - foreach ($this->storedObjectVersionRepository->findIdsByVersionsOlderThanDateAndNotLastVersion($deleteBeforeDate) as $id) { + foreach ($this->storedObjectVersionRepository->findIdsByVersionsOlderThanDateAndNotLastVersionAndNotPointInTime($deleteBeforeDate) as $id) { $this->messageBus->dispatch(new RemoveOldVersionMessage($id)); $maxDeleted = max($maxDeleted, $id); } diff --git a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveOldVersionMessageHandler.php b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveOldVersionMessageHandler.php index 69c9f283d..ea3e37be7 100644 --- a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveOldVersionMessageHandler.php +++ b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveOldVersionMessageHandler.php @@ -18,6 +18,7 @@ use Chill\DocStoreBundle\Service\StoredObjectManagerInterface; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Symfony\Component\Clock\ClockInterface; +use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException; use Symfony\Component\Messenger\Handler\MessageHandlerInterface; /** @@ -49,13 +50,18 @@ final readonly class RemoveOldVersionMessageHandler implements MessageHandlerInt $this->logger->info(self::LOG_PREFIX.'Received one message', ['storedObjectVersionId' => $message->storedObjectVersionId]); $storedObjectVersion = $this->storedObjectVersionRepository->find($message->storedObjectVersionId); - $storedObject = $storedObjectVersion->getStoredObject(); if (null === $storedObjectVersion) { $this->logger->error(self::LOG_PREFIX.'StoredObjectVersion not found in database', ['storedObjectVersionId' => $message->storedObjectVersionId]); throw new \RuntimeException('StoredObjectVersion not found with id '.$message->storedObjectVersionId); } + if ($storedObjectVersion->hasPointInTimes()) { + throw new UnrecoverableMessageHandlingException('the stored object version is now associated with a point in time'); + } + + $storedObject = $storedObjectVersion->getStoredObject(); + $this->storedObjectManager->delete($storedObjectVersion); // to ensure an immediate deletion $this->entityManager->remove($storedObjectVersion); diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Service/Repository/StoredObjectVersionRepositoryTest.php b/src/Bundle/ChillDocStoreBundle/Tests/Service/Repository/StoredObjectVersionRepositoryTest.php index ace122bea..4b134a1fb 100644 --- a/src/Bundle/ChillDocStoreBundle/Tests/Service/Repository/StoredObjectVersionRepositoryTest.php +++ b/src/Bundle/ChillDocStoreBundle/Tests/Service/Repository/StoredObjectVersionRepositoryTest.php @@ -35,7 +35,7 @@ class StoredObjectVersionRepositoryTest extends KernelTestCase $repository = new StoredObjectVersionRepository($this->entityManager); // get old version, to get a chance to get one - $actual = $repository->findIdsByVersionsOlderThanDateAndNotLastVersion(new \DateTimeImmutable('1970-01-01')); + $actual = $repository->findIdsByVersionsOlderThanDateAndNotLastVersionAndNotPointInTime(new \DateTimeImmutable('1970-01-01')); self::assertIsIterable($actual); self::assertContainsOnly('int', $actual); diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectCleaner/RemoveOldVersionCronJobTest.php b/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectCleaner/RemoveOldVersionCronJobTest.php index 937253c81..df75eea93 100644 --- a/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectCleaner/RemoveOldVersionCronJobTest.php +++ b/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectCleaner/RemoveOldVersionCronJobTest.php @@ -46,7 +46,7 @@ class RemoveOldVersionCronJobTest extends KernelTestCase $clock = new MockClock(new \DateTimeImmutable('2024-01-01 00:00:00', new \DateTimeZone('+00:00'))); $repository = $this->createMock(StoredObjectVersionRepository::class); $repository->expects($this->once()) - ->method('findIdsByVersionsOlderThanDateAndNotLastVersion') + ->method('findIdsByVersionsOlderThanDateAndNotLastVersionAndNotPointInTime') ->with(new \DateTime('2023-10-03 00:00:00', new \DateTimeZone('+00:00'))) ->willReturnCallback(function ($arg) { yield 1; From 1ddd283f26bf0ab545a28b7a41becebc030efc05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Tue, 10 Sep 2024 14:19:14 +0200 Subject: [PATCH 50/60] Add signer type differentiation for workflows Added a method to determine if the signer is a 'person' or 'user'. Updated the signature template to handle both types accordingly, ensuring the correct entity type is displayed in workflow signatures. --- .../Workflow/EntityWorkflowStepSignature.php | 12 +++++++++++ .../views/Workflow/_signature.html.twig | 20 +++++++++++++------ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php index 527ede0ef..7f6c5cdda 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php @@ -140,4 +140,16 @@ class EntityWorkflowStepSignature implements TrackCreationInterface, TrackUpdate { return EntityWorkflowSignatureStateEnum::SIGNED == $this->getState(); } + + /** + * @return 'person'|'user' + */ + public function getSignerKind(): string + { + if ($this->personSigner instanceof Person) { + return 'person'; + } + + return 'user'; + } } diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig index 68b6f4274..50172089c 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_signature.html.twig @@ -4,12 +4,20 @@ {% for s in signatures %}
      - {% include '@ChillMain/OnTheFly/_insert_vue_onthefly.html.twig' with { - action: 'show', displayBadge: true, - targetEntity: { name: 'person', id: s.signer.id }, - buttonText: s.signer|chill_entity_render_string, - isDead: s.signer.deathDate is not null - } %} + {% if s.signerKind == 'person' %} + {% include '@ChillMain/OnTheFly/_insert_vue_onthefly.html.twig' with { + action: 'show', displayBadge: true, + targetEntity: { name: 'person', id: s.signer.id }, + buttonText: s.signer|chill_entity_render_string, + isDead: s.signer.deathDate is not null + } %} + {% else %} + {% include '@ChillMain/OnTheFly/_insert_vue_onthefly.html.twig' with { + action: 'show', displayBadge: true, + targetEntity: { name: 'user', id: s.signer.id }, + buttonText: s.signer|chill_entity_render_string, + } %} + {% endif %}
      {% if s.isSigned %} From a60ea0e0661674ee56bde9ce823a3d5b8985e380 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Tue, 10 Sep 2024 14:28:47 +0200 Subject: [PATCH 51/60] Add StoredObjectToPdfConverter service and unit tests Introduced the StoredObjectToPdfConverter service to handle conversion of stored objects to PDF format. Added unit tests to ensure proper functionality, including versioning and exception handling. --- .../Service/StoredObjectToPdfConverter.php | 75 +++++++++++++++++++ .../StoredObjectToPdfConverterTest.php | 61 +++++++++++++++ 2 files changed, 136 insertions(+) create mode 100644 src/Bundle/ChillDocStoreBundle/Service/StoredObjectToPdfConverter.php create mode 100644 src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectToPdfConverterTest.php diff --git a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectToPdfConverter.php b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectToPdfConverter.php new file mode 100644 index 000000000..abfcca4cc --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectToPdfConverter.php @@ -0,0 +1,75 @@ +mimeTypes->getMimeTypes($convertTo)[0] ?? null; + + if (null === $newMimeType) { + throw new \UnexpectedValueException(sprintf('could not find a preferred mime type for conversion to %s', $convertTo)); + } + + $currentVersion = $storedObject->getCurrentVersion(); + + if ($currentVersion->getType() === $newMimeType) { + throw new \UnexpectedValueException('Already at the same mime type'); + } + + $content = $this->storedObjectManager->read($currentVersion); + + try { + $converted = $this->wopiConverter->convert($lang, $content, $newMimeType, $convertTo); + } catch (\RuntimeException $e) { + throw new \RuntimeException('could not store a new version for document', previous: $e); + } + + $pointInTime = new StoredObjectPointInTime($currentVersion, StoredObjectPointInTimeReasonEnum::KEEP_BEFORE_CONVERSION); + $version = $this->storedObjectManager->write($storedObject, $converted, $newMimeType); + + return [$pointInTime, $version]; + } +} diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectToPdfConverterTest.php b/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectToPdfConverterTest.php new file mode 100644 index 000000000..da3ed9210 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectToPdfConverterTest.php @@ -0,0 +1,61 @@ +registerVersion(type: 'text/html'); + + $storedObjectManager = $this->prophesize(StoredObjectManagerInterface::class); + $storedObjectManager->read($currentVersion)->willReturn('1234'); + $storedObjectManager->write($storedObject, '5678', 'application/pdf')->shouldBeCalled() + ->will(function ($args) { + /** @var StoredObject $storedObject */ + $storedObject = $args[0]; + + return $storedObject->registerVersion(type: $args[2]); + }); + + $converter = $this->prophesize(WopiConverter::class); + $converter->convert('fr', '1234', 'application/pdf', 'pdf')->shouldBeCalled() + ->willReturn('5678'); + + $converter = new StoredObjectToPdfConverter($storedObjectManager->reveal(), $converter->reveal(), MimeTypes::getDefault()); + + $actual = $converter->addConvertedVersion($storedObject, 'fr'); + + self::assertIsArray($actual); + self::assertInstanceOf(StoredObjectPointInTime::class, $actual[0]); + self::assertSame($currentVersion, $actual[0]->getObjectVersion()); + self::assertInstanceOf(StoredObjectVersion::class, $actual[1]); + } +} From 941444b7d5b0d8b320f8c4bb061a0935748d4e6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Tue, 10 Sep 2024 14:29:03 +0200 Subject: [PATCH 52/60] Add event subscriber to convert docs to PDF before signature Introduce ConvertToPdfBeforeSignatureStepEventSubscriber to convert documents to PDF when reaching a signature step in the workflow. Includes tests to ensure the conversion process only triggers when necessary. --- ...BeforeSignatureStepEventSubscriberTest.php | 208 ++++++++++++++++++ ...oPdfBeforeSignatureStepEventSubscriber.php | 75 +++++++ 2 files changed, 283 insertions(+) create mode 100644 src/Bundle/ChillDocStoreBundle/Tests/Workflow/ConvertToPdfBeforeSignatureStepEventSubscriberTest.php create mode 100644 src/Bundle/ChillDocStoreBundle/Workflow/ConvertToPdfBeforeSignatureStepEventSubscriber.php diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Workflow/ConvertToPdfBeforeSignatureStepEventSubscriberTest.php b/src/Bundle/ChillDocStoreBundle/Tests/Workflow/ConvertToPdfBeforeSignatureStepEventSubscriberTest.php new file mode 100644 index 000000000..572cb192c --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Tests/Workflow/ConvertToPdfBeforeSignatureStepEventSubscriberTest.php @@ -0,0 +1,208 @@ +registerVersion(); + + $converter = $this->prophesize(StoredObjectToPdfConverter::class); + $converter->addConvertedVersion($storedObject, 'fr', 'pdf') + ->shouldBeCalledOnce() + ->will(function ($args) { + /** @var StoredObject $storedObject */ + $storedObject = $args[0]; + + $pointInTime = new StoredObjectPointInTime($storedObject->getCurrentVersion(), StoredObjectPointInTimeReasonEnum::KEEP_BEFORE_CONVERSION); + $newVersion = $storedObject->registerVersion(filename: 'next'); + + return [$pointInTime, $newVersion]; + }); + + $entityWorkflowManager = $this->prophesize(EntityWorkflowManager::class); + $entityWorkflowManager->getAssociatedStoredObject($entityWorkflow)->willReturn($storedObject); + + $request = new Request(); + $request->setLocale('fr'); + $stack = new RequestStack(); + $stack->push($request); + + $eventSubscriber = new ConvertToPdfBeforeSignatureStepEventSubscriber($entityWorkflowManager->reveal(), $converter->reveal(), $stack); + + $registry = $this->buildRegistry($eventSubscriber); + $workflow = $registry->get($entityWorkflow, 'dummy'); + + $dto = new WorkflowTransitionContextDTO($entityWorkflow); + $workflow->apply($entityWorkflow, 'to_signature', ['context' => $dto, 'transition' => 'to_signature', 'transitionAt' => new \DateTimeImmutable('now')]); + + self::assertEquals('signature', $entityWorkflow->getStep()); + self::assertNotSame($previousVersion, $storedObject->getCurrentVersion()); + self::assertTrue($previousVersion->hasPointInTimes()); + self::assertCount(2, $storedObject->getVersions()); + self::assertEquals('next', $storedObject->getCurrentVersion()->getFilename()); + } + + public function testConvertToPdfBeforeSignatureStepEventSubscriberToNotASignatureStep(): void + { + $entityWorkflow = new EntityWorkflow(); + $storedObject = new StoredObject(); + $previousVersion = $storedObject->registerVersion(); + + $converter = $this->prophesize(StoredObjectToPdfConverter::class); + $converter->addConvertedVersion($storedObject, 'fr', 'pdf') + ->shouldNotBeCalled(); + + $entityWorkflowManager = $this->prophesize(EntityWorkflowManager::class); + $entityWorkflowManager->getAssociatedStoredObject($entityWorkflow)->willReturn($storedObject); + + $request = new Request(); + $request->setLocale('fr'); + $stack = new RequestStack(); + $stack->push($request); + + $eventSubscriber = new ConvertToPdfBeforeSignatureStepEventSubscriber($entityWorkflowManager->reveal(), $converter->reveal(), $stack); + + $registry = $this->buildRegistry($eventSubscriber); + $workflow = $registry->get($entityWorkflow, 'dummy'); + + $dto = new WorkflowTransitionContextDTO($entityWorkflow); + $workflow->apply($entityWorkflow, 'to_something', ['context' => $dto, 'transition' => 'to_something', 'transitionAt' => new \DateTimeImmutable('now')]); + + self::assertEquals('something', $entityWorkflow->getStep()); + self::assertSame($previousVersion, $storedObject->getCurrentVersion()); + self::assertFalse($previousVersion->hasPointInTimes()); + self::assertCount(1, $storedObject->getVersions()); + } + + public function testConvertToPdfBeforeSignatureStepEventSubscriberToSignatureAlreadyAPdf(): void + { + $entityWorkflow = new EntityWorkflow(); + $storedObject = new StoredObject(); + $previousVersion = $storedObject->registerVersion(type: 'application/pdf'); + + $converter = $this->prophesize(StoredObjectToPdfConverter::class); + $converter->addConvertedVersion($storedObject, 'fr', 'pdf') + ->shouldNotBeCalled(); + + $entityWorkflowManager = $this->prophesize(EntityWorkflowManager::class); + $entityWorkflowManager->getAssociatedStoredObject($entityWorkflow)->willReturn($storedObject); + + $request = new Request(); + $request->setLocale('fr'); + $stack = new RequestStack(); + $stack->push($request); + + $eventSubscriber = new ConvertToPdfBeforeSignatureStepEventSubscriber($entityWorkflowManager->reveal(), $converter->reveal(), $stack); + + $registry = $this->buildRegistry($eventSubscriber); + $workflow = $registry->get($entityWorkflow, 'dummy'); + + $dto = new WorkflowTransitionContextDTO($entityWorkflow); + $workflow->apply($entityWorkflow, 'to_signature', ['context' => $dto, 'transition' => 'to_signature', 'transitionAt' => new \DateTimeImmutable('now')]); + + self::assertEquals('signature', $entityWorkflow->getStep()); + self::assertSame($previousVersion, $storedObject->getCurrentVersion()); + self::assertFalse($previousVersion->hasPointInTimes()); + self::assertCount(1, $storedObject->getVersions()); + } + + public function testConvertToPdfBeforeSignatureStepEventSubscriberToSignatureWithNoStoredObject(): void + { + $entityWorkflow = new EntityWorkflow(); + + $converter = $this->prophesize(StoredObjectToPdfConverter::class); + $converter->addConvertedVersion(Argument::type(StoredObject::class), 'fr', 'pdf') + ->shouldNotBeCalled(); + + $entityWorkflowManager = $this->prophesize(EntityWorkflowManager::class); + $entityWorkflowManager->getAssociatedStoredObject($entityWorkflow)->willReturn(null); + + $request = new Request(); + $request->setLocale('fr'); + $stack = new RequestStack(); + $stack->push($request); + + $eventSubscriber = new ConvertToPdfBeforeSignatureStepEventSubscriber($entityWorkflowManager->reveal(), $converter->reveal(), $stack); + + $registry = $this->buildRegistry($eventSubscriber); + $workflow = $registry->get($entityWorkflow, 'dummy'); + + $dto = new WorkflowTransitionContextDTO($entityWorkflow); + $workflow->apply($entityWorkflow, 'to_signature', ['context' => $dto, 'transition' => 'to_signature', 'transitionAt' => new \DateTimeImmutable('now')]); + + self::assertEquals('signature', $entityWorkflow->getStep()); + } + + private function buildRegistry(EventSubscriberInterface $eventSubscriber): Registry + { + $builder = new DefinitionBuilder(); + $builder + ->setInitialPlaces('initial') + ->addPlaces(['initial', 'signature', 'something']) + ->addTransition(new Transition('to_something', 'initial', 'something')) + ->addTransition(new Transition('to_signature', 'initial', 'signature')); + + $metadataStore = new InMemoryMetadataStore([], ['signature' => ['isSignature' => ['user']]]); + $builder->setMetadataStore($metadataStore); + + $eventDispatcher = new EventDispatcher(); + $eventDispatcher->addSubscriber($eventSubscriber); + + $workflow = new Workflow($builder->build(), new EntityWorkflowMarkingStore(), $eventDispatcher, 'dummy'); + + $supports = new class () implements WorkflowSupportStrategyInterface { + public function supports(WorkflowInterface $workflow, object $subject): bool + { + return true; + } + }; + + $registry = new Registry(); + $registry->addWorkflow($workflow, $supports); + + return $registry; + } +} diff --git a/src/Bundle/ChillDocStoreBundle/Workflow/ConvertToPdfBeforeSignatureStepEventSubscriber.php b/src/Bundle/ChillDocStoreBundle/Workflow/ConvertToPdfBeforeSignatureStepEventSubscriber.php new file mode 100644 index 000000000..0d5fe9323 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Workflow/ConvertToPdfBeforeSignatureStepEventSubscriber.php @@ -0,0 +1,75 @@ + 'convertToPdfBeforeSignatureStepEvent', + ]; + } + + public function convertToPdfBeforeSignatureStepEvent(CompletedEvent $event): void + { + $entityWorkflow = $event->getSubject(); + if (!$entityWorkflow instanceof EntityWorkflow) { + return; + } + + $tos = $event->getTransition()->getTos(); + $workflow = $event->getWorkflow(); + $metadataStore = $workflow->getMetadataStore(); + + foreach ($tos as $to) { + $metadata = $metadataStore->getPlaceMetadata($to); + if (array_key_exists('isSignature', $metadata) && 0 < count($metadata['isSignature'])) { + $this->convertToPdf($entityWorkflow); + + return; + } + } + } + + private function convertToPdf(EntityWorkflow $entityWorkflow): void + { + $storedObject = $this->entityWorkflowManager->getAssociatedStoredObject($entityWorkflow); + + if (null === $storedObject) { + return; + } + + if ('application/pdf' === $storedObject->getCurrentVersion()->getType()) { + return; + } + + $this->storedObjectToPdfConverter->addConvertedVersion($storedObject, $this->requestStack->getCurrentRequest()->getLocale(), 'pdf'); + } +} From 1197a46f5f934d4ffb9c1cf449c5e47f9de6c542 Mon Sep 17 00:00:00 2001 From: nobohan Date: Thu, 18 Jul 2024 17:13:47 +0200 Subject: [PATCH 53/60] Refactor PDF signature handling and add signature state changer Simplified PdfSignedMessageHandler by delegating signature state changes to a new SignatureStepStateChanger class. Added utility method to EntityWorkflowStepSignature for checking pending signatures and created new test cases for the SignatureStepStateChanger. --- .../BaseSigner/PdfSignedMessageHandler.php | 9 +- .../PdfSignedMessageHandlerTest.php | 10 +- .../Workflow/EntityWorkflowStepSignature.php | 26 ++++ .../SignatureStepStateChangerTest.php | 145 ++++++++++++++++++ .../Workflow/SignatureStepStateChanger.php | 96 ++++++++++++ 5 files changed, 276 insertions(+), 10 deletions(-) create mode 100644 src/Bundle/ChillMainBundle/Tests/Workflow/SignatureStepStateChangerTest.php create mode 100644 src/Bundle/ChillMainBundle/Workflow/SignatureStepStateChanger.php diff --git a/src/Bundle/ChillDocStoreBundle/Service/Signature/Driver/BaseSigner/PdfSignedMessageHandler.php b/src/Bundle/ChillDocStoreBundle/Service/Signature/Driver/BaseSigner/PdfSignedMessageHandler.php index 720bc000f..e97781a15 100644 --- a/src/Bundle/ChillDocStoreBundle/Service/Signature/Driver/BaseSigner/PdfSignedMessageHandler.php +++ b/src/Bundle/ChillDocStoreBundle/Service/Signature/Driver/BaseSigner/PdfSignedMessageHandler.php @@ -12,12 +12,11 @@ declare(strict_types=1); namespace Chill\DocStoreBundle\Service\Signature\Driver\BaseSigner; use Chill\DocStoreBundle\Service\StoredObjectManagerInterface; -use Chill\MainBundle\Entity\Workflow\EntityWorkflowSignatureStateEnum; use Chill\MainBundle\Repository\Workflow\EntityWorkflowStepSignatureRepository; use Chill\MainBundle\Workflow\EntityWorkflowManager; +use Chill\MainBundle\Workflow\SignatureStepStateChanger; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; -use Symfony\Component\Clock\ClockInterface; use Symfony\Component\Messenger\Handler\MessageHandlerInterface; final readonly class PdfSignedMessageHandler implements MessageHandlerInterface @@ -33,7 +32,7 @@ final readonly class PdfSignedMessageHandler implements MessageHandlerInterface private StoredObjectManagerInterface $storedObjectManager, private EntityWorkflowStepSignatureRepository $entityWorkflowStepSignatureRepository, private EntityManagerInterface $entityManager, - private ClockInterface $clock, + private SignatureStepStateChanger $signatureStepStateChanger, ) {} public function __invoke(PdfSignedMessage $message): void @@ -54,8 +53,8 @@ final readonly class PdfSignedMessageHandler implements MessageHandlerInterface $this->storedObjectManager->write($storedObject, $message->content); - $signature->setState(EntityWorkflowSignatureStateEnum::SIGNED)->setStateDate($this->clock->now()); - $signature->setZoneSignatureIndex($message->signatureZoneIndex); + $this->signatureStepStateChanger->markSignatureAsSigned($signature, $message->signatureZoneIndex); + $this->entityManager->flush(); $this->entityManager->clear(); } diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Service/Signature/Driver/BaseSigner/PdfSignedMessageHandlerTest.php b/src/Bundle/ChillDocStoreBundle/Tests/Service/Signature/Driver/BaseSigner/PdfSignedMessageHandlerTest.php index 471dc8f9a..fed1b1274 100644 --- a/src/Bundle/ChillDocStoreBundle/Tests/Service/Signature/Driver/BaseSigner/PdfSignedMessageHandlerTest.php +++ b/src/Bundle/ChillDocStoreBundle/Tests/Service/Signature/Driver/BaseSigner/PdfSignedMessageHandlerTest.php @@ -20,12 +20,12 @@ use Chill\MainBundle\Entity\Workflow\EntityWorkflow; use Chill\MainBundle\Entity\Workflow\EntityWorkflowStepSignature; use Chill\MainBundle\Repository\Workflow\EntityWorkflowStepSignatureRepository; use Chill\MainBundle\Workflow\EntityWorkflowManager; +use Chill\MainBundle\Workflow\SignatureStepStateChanger; use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO; use Chill\PersonBundle\Entity\Person; use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; -use Symfony\Component\Clock\MockClock; /** * @internal @@ -45,6 +45,9 @@ class PdfSignedMessageHandlerTest extends TestCase $entityWorkflow->setStep('new_step', $dto, 'new_transition', new \DateTimeImmutable(), new User()); $step = $entityWorkflow->getCurrentStep(); $signature = $step->getSignatures()->first(); + $stateChanger = $this->createMock(SignatureStepStateChanger::class); + $stateChanger->expects(self::once())->method('markSignatureAsSigned') + ->with($signature, 99); $handler = new PdfSignedMessageHandler( new NullLogger(), @@ -52,15 +55,12 @@ class PdfSignedMessageHandlerTest extends TestCase $this->buildStoredObjectManager($storedObject, $expectedContent = '1234'), $this->buildSignatureRepository($signature), $this->buildEntityManager(true), - new MockClock('now'), + $stateChanger, ); // we simply call the handler. The mocked StoredObjectManager will check that the "write" method is invoked once // with the content "1234" $handler(new PdfSignedMessage(10, 99, $expectedContent)); - - self::assertEquals('signed', $signature->getState()->value); - self::assertEquals(99, $signature->getZoneSignatureIndex()); } private function buildSignatureRepository(EntityWorkflowStepSignature $signature): EntityWorkflowStepSignatureRepository diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php index 7f6c5cdda..a0fe1755c 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php @@ -141,6 +141,32 @@ class EntityWorkflowStepSignature implements TrackCreationInterface, TrackUpdate return EntityWorkflowSignatureStateEnum::SIGNED == $this->getState(); } + public function isPending(): bool + { + return EntityWorkflowSignatureStateEnum::PENDING == $this->getState(); + } + + /** + * Checks whether all signatures associated with a given workflow step are not pending. + * + * Iterates over each signature in the provided workflow step, and returns false if any signature + * is found to be pending. If all signatures are not pending, returns true. + * + * @param EntityWorkflowStep $step the workflow step whose signatures are to be checked + * + * @return bool true if all signatures are not pending, false otherwise + */ + public static function isAllSignatureNotPendingForStep(EntityWorkflowStep $step): bool + { + foreach ($step->getSignatures() as $signature) { + if ($signature->isPending()) { + return false; + } + } + + return true; + } + /** * @return 'person'|'user' */ diff --git a/src/Bundle/ChillMainBundle/Tests/Workflow/SignatureStepStateChangerTest.php b/src/Bundle/ChillMainBundle/Tests/Workflow/SignatureStepStateChangerTest.php new file mode 100644 index 000000000..5eb82bab9 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Tests/Workflow/SignatureStepStateChangerTest.php @@ -0,0 +1,145 @@ +setWorkflowName('dummy'); + $registry = $this->buildRegistry(); + $workflow = $registry->get($entityWorkflow, 'dummy'); + $clock = new MockClock(); + $user = new User(); + $changer = new SignatureStepStateChanger($registry, $clock); + + // move it to signature + $dto = new WorkflowTransitionContextDTO($entityWorkflow); + $dto->futurePersonSignatures = [new Person(), new Person()]; + $workflow->apply($entityWorkflow, 'to_signature', ['context' => $dto, 'transitionAt' => $clock->now(), + 'byUser' => $user, 'transition' => 'to_signature']); + + // get the signature created + $signatures = $entityWorkflow->getCurrentStep()->getSignatures(); + + if (2 !== count($signatures)) { + throw new \LogicException('there should have 2 signatures at this step'); + } + + // we mark the first signature as signed + $changer->markSignatureAsSigned($signatures[0], 1); + + self::assertEquals('signature', $entityWorkflow->getStep(), 'there should have any change in the entity workflow step'); + self::assertEquals(EntityWorkflowSignatureStateEnum::SIGNED, $signatures[0]->getState()); + self::assertEquals(1, $signatures[0]->getZoneSignatureIndex()); + self::assertNotNull($signatures[0]->getStateDate()); + + + // we mark the second signature as signed + $changer->markSignatureAsSigned($signatures[1], 2); + self::assertEquals(EntityWorkflowSignatureStateEnum::SIGNED, $signatures[1]->getState()); + self::assertEquals('post-signature', $entityWorkflow->getStep(), 'the entity workflow step should be post-signature'); + self::assertContains($user, $entityWorkflow->getCurrentStep()->getAllDestUser()); + self::assertEquals(2, $signatures[1]->getZoneSignatureIndex()); + self::assertNotNull($signatures[1]->getStateDate()); + } + + public function testMarkSignatureAsSignedScenarioWithoutRequiredMetadata() + { + $entityWorkflow = new EntityWorkflow(); + $entityWorkflow->setWorkflowName('dummy'); + $registry = $this->buildRegistry(); + $workflow = $registry->get($entityWorkflow, 'dummy'); + $clock = new MockClock(); + $user = new User(); + $changer = new SignatureStepStateChanger($registry, $clock); + + // move it to signature + $dto = new WorkflowTransitionContextDTO($entityWorkflow); + $dto->futurePersonSignatures = [new Person()]; + $workflow->apply($entityWorkflow, 'to_signature-without-metadata', ['context' => $dto, 'transitionAt' => $clock->now(), + 'byUser' => $user, 'transition' => 'to_signature-without-metadata']); + + // get the signature created + $signatures = $entityWorkflow->getCurrentStep()->getSignatures(); + + if (1 !== count($signatures)) { + throw new \LogicException('there should have 2 signatures at this step'); + } + + // we mark the first signature as signed + $changer->markSignatureAsSigned($signatures[0], 1); + + self::assertEquals('signature-without-metadata', $entityWorkflow->getStep(), 'there should have any change in the entity workflow step'); + self::assertEquals(EntityWorkflowSignatureStateEnum::SIGNED, $signatures[0]->getState()); + self::assertEquals(1, $signatures[0]->getZoneSignatureIndex()); + self::assertNotNull($signatures[0]->getStateDate()); + } + + private function buildRegistry(): Registry + { + $builder = new DefinitionBuilder(); + $builder + ->setInitialPlaces('initial') + ->addPlaces(['initial', 'signature', 'signature-without-metadata', 'post-signature']) + ->addTransition(new Transition('to_signature', 'initial', 'signature')) + ->addTransition(new Transition('to_signature-without-metadata', 'initial', 'signature-without-metadata')) + ->addTransition(new Transition('to_post-signature', 'signature', 'post-signature')) + ->addTransition(new Transition('to_post-signature_2', 'signature-without-metadata', 'post-signature')) + ; + + $metadata = new InMemoryMetadataStore( + [], + [ + 'signature' => ['onSignatureCompleted' => ['transitionName' => 'to_post-signature']], + ] + ); + $builder->setMetadataStore($metadata); + + $workflow = new Workflow($builder->build(), new EntityWorkflowMarkingStore(), name: 'dummy'); + $registry = new Registry(); + $registry->addWorkflow( + $workflow, + new class () implements WorkflowSupportStrategyInterface { + public function supports(WorkflowInterface $workflow, object $subject): bool + { + return true; + } + } + ); + + return $registry; + } +} diff --git a/src/Bundle/ChillMainBundle/Workflow/SignatureStepStateChanger.php b/src/Bundle/ChillMainBundle/Workflow/SignatureStepStateChanger.php new file mode 100644 index 000000000..efba00b02 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Workflow/SignatureStepStateChanger.php @@ -0,0 +1,96 @@ +setState(EntityWorkflowSignatureStateEnum::SIGNED) + ->setZoneSignatureIndex($atIndex) + ->setStateDate($this->clock->now()) + ; + + if (!EntityWorkflowStepSignature::isAllSignatureNotPendingForStep($signature->getStep())) { + return; + } + + $entityWorkflow = $signature->getStep()->getEntityWorkflow(); + $workflow = $this->registry->get($entityWorkflow, $entityWorkflow->getWorkflowName()); + $metadataStore = $workflow->getMetadataStore(); + + // find a transition + $marking = $workflow->getMarking($entityWorkflow); + $places = $marking->getPlaces(); + + $transition = null; + foreach ($places as $place => $int) { + $metadata = $metadataStore->getPlaceMetadata($place); + if (array_key_exists('onSignatureCompleted', $metadata)) { + $transition = $metadata['onSignatureCompleted']['transitionName']; + } + } + + if (null === $transition) { + return; + } + + $previousUser = $this->getPreviousSender($signature->getStep()); + + if (null === $previousUser) { + return; + } + + $transitionDto = new WorkflowTransitionContextDTO($entityWorkflow); + $transitionDto->futureDestUsers[] = $previousUser; + + $workflow->apply($entityWorkflow, $transition, [ + 'context' => $transitionDto, + 'transitionAt' => $this->clock->now(), + 'transition' => $transition, + ]); + } + + private function getPreviousSender(EntityWorkflowStep $entityWorkflowStep): ?User + { + $stepsChained = $entityWorkflowStep->getEntityWorkflow()->getStepsChained(); + + foreach ($stepsChained as $stepChained) { + if ($stepChained === $entityWorkflowStep) { + if (null === $previous = $stepChained->getPrevious()) { + return null; + } + + if (null !== $previousUser = $previous->getTransitionBy()) { + return $previousUser; + } + + return $this->getPreviousSender($previous); + } + } + + throw new \LogicException('no same step found'); + } +} From 70671dadac8d93bd2e17e4af55f6d98819285ba9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 11 Sep 2024 18:29:44 +0200 Subject: [PATCH 54/60] Refactor workflow guard logic and add internal methods Removed guard logic from EntityWorkflowTransitionEventSubscriber and created a new EntityWorkflowGuardTransition class for separation of concerns. Marked several setter methods in EntityWorkflowStepSignature as internal to guide proper usage. Added comprehensive tests to ensure the new guard logic functions correctly. --- .../Workflow/EntityWorkflowStepSignature.php | 15 ++ .../EntityWorkflowGuardTransitionTest.php | 168 ++++++++++++++++++ .../EntityWorkflowGuardTransition.php | 105 +++++++++++ ...ntityWorkflowTransitionEventSubscriber.php | 47 +---- 4 files changed, 291 insertions(+), 44 deletions(-) create mode 100644 src/Bundle/ChillMainBundle/Tests/Workflow/EventSubscriber/EntityWorkflowGuardTransitionTest.php create mode 100644 src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowGuardTransition.php diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php index a0fe1755c..5b659d844 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflowStepSignature.php @@ -105,6 +105,11 @@ class EntityWorkflowStepSignature implements TrackCreationInterface, TrackUpdate return $this->state; } + /** + * @return $this + * + * @internal You should not use this method directly, use @see{Chill\MainBundle\Workflow\SignatureStepStateChanger} instead + */ public function setState(EntityWorkflowSignatureStateEnum $state): EntityWorkflowStepSignature { $this->state = $state; @@ -117,6 +122,11 @@ class EntityWorkflowStepSignature implements TrackCreationInterface, TrackUpdate return $this->stateDate; } + /** + * @return $this + * + * @internal You should not use this method directly, use @see{Chill\MainBundle\Workflow\SignatureStepStateChanger} instead + */ public function setStateDate(?\DateTimeImmutable $stateDate): EntityWorkflowStepSignature { $this->stateDate = $stateDate; @@ -129,6 +139,11 @@ class EntityWorkflowStepSignature implements TrackCreationInterface, TrackUpdate return $this->zoneSignatureIndex; } + /** + * @return $this + * + * @internal You should not use this method directly, use @see{Chill\MainBundle\Workflow\SignatureStepStateChanger} instead + */ public function setZoneSignatureIndex(?int $zoneSignatureIndex): EntityWorkflowStepSignature { $this->zoneSignatureIndex = $zoneSignatureIndex; diff --git a/src/Bundle/ChillMainBundle/Tests/Workflow/EventSubscriber/EntityWorkflowGuardTransitionTest.php b/src/Bundle/ChillMainBundle/Tests/Workflow/EventSubscriber/EntityWorkflowGuardTransitionTest.php new file mode 100644 index 000000000..bccd6cf9e --- /dev/null +++ b/src/Bundle/ChillMainBundle/Tests/Workflow/EventSubscriber/EntityWorkflowGuardTransitionTest.php @@ -0,0 +1,168 @@ +setInitialPlaces(['initial']) + ->addPlaces(['initial', 'intermediate', 'step1', 'step2', 'step3']) + ->addTransition(new Transition('intermediate', 'initial', 'intermediate')) + ->addTransition($transition1 = new Transition('transition1', 'intermediate', 'step1')) + ->addTransition($transition2 = new Transition('transition2', 'intermediate', 'step2')) + ->addTransition($transition3 = new Transition('transition3', 'intermediate', 'step3')) + ; + + $transitionMetadata = new \SplObjectStorage(); + $transitionMetadata->attach($transition1, ['transitionGuard' => 'only-dest']); + $transitionMetadata->attach($transition2, ['transitionGuard' => 'only-dest+system']); + $transitionMetadata->attach($transition3, ['transitionGuard' => 'system']); + + $builder->setMetadataStore(new InMemoryMetadataStore(transitionsMetadata: $transitionMetadata)); + + if (null !== $eventSubscriber) { + $eventDispatcher = new EventDispatcher(); + $eventDispatcher->addSubscriber($eventSubscriber); + } + + $workflow = new Workflow($builder->build(), new EntityWorkflowMarkingStore(), $eventDispatcher ?? null, 'dummy'); + + $registry = new Registry(); + $registry->addWorkflow( + $workflow, + new class () implements WorkflowSupportStrategyInterface { + public function supports(WorkflowInterface $workflow, object $subject): bool + { + return true; + } + } + ); + + return $registry; + } + + /** + * @dataProvider provideBlockingTransition + */ + public function testTransitionGuardBlocked(EntityWorkflow $entityWorkflow, string $transition, ?User $user, string $uuid): void + { + $userRender = $this->prophesize(UserRender::class); + $userRender->renderString(Argument::type(User::class), [])->willReturn('user-as-string'); + $security = $this->prophesize(Security::class); + $security->getUser()->willReturn($user); + + $transitionGuard = new EntityWorkflowGuardTransition($userRender->reveal(), $security->reveal()); + $registry = self::buildRegistry($transitionGuard); + + $workflow = $registry->get($entityWorkflow, 'dummy'); + + $context = new WorkflowTransitionContextDTO($entityWorkflow); + + self::expectException(NotEnabledTransitionException::class); + try { + $workflow->apply($entityWorkflow, $transition, ['context' => $context, 'byUser' => $user, 'transition' => $transition, 'transitionAt' => new \DateTimeImmutable('now')]); + } catch (NotEnabledTransitionException $e) { + $list = $e->getTransitionBlockerList(); + + self::assertEquals(1, $list->count()); + $list = iterator_to_array($list->getIterator()); + self::assertEquals($uuid, $list[0]->getCode()); + + throw $e; + } + } + + /** + * @dataProvider provideValidTransition + */ + public function testTransitionGuardValid(EntityWorkflow $entityWorkflow, string $transition, ?User $user, string $newStep): void + { + $userRender = $this->prophesize(UserRender::class); + $userRender->renderString(Argument::type(User::class), [])->willReturn('user-as-string'); + $security = $this->prophesize(Security::class); + $security->getUser()->willReturn($user); + + $transitionGuard = new EntityWorkflowGuardTransition($userRender->reveal(), $security->reveal()); + $registry = self::buildRegistry($transitionGuard); + + $workflow = $registry->get($entityWorkflow, 'dummy'); + $context = new WorkflowTransitionContextDTO($entityWorkflow); + + $workflow->apply($entityWorkflow, $transition, ['context' => $context, 'byUser' => $user, 'transition' => $transition, 'transitionAt' => new \DateTimeImmutable('now')]); + + self::assertEquals($newStep, $entityWorkflow->getStep()); + } + + public static function provideBlockingTransition(): iterable + { + yield [self::buildEntityWorkflow([new User()]), 'transition1', new User(), 'f3eeb57c-7532-11ec-9495-e7942a2ac7bc']; + yield [self::buildEntityWorkflow([]), 'transition1', null, 'd9e39a18-704c-11ef-b235-8fe0619caee7']; + yield [self::buildEntityWorkflow([new User()]), 'transition1', null, 'd9e39a18-704c-11ef-b235-8fe0619caee7']; + yield [self::buildEntityWorkflow([$user = new User()]), 'transition3', $user, '5b6b95e0-704d-11ef-a5a9-4b6fc11a8eeb']; + } + + public static function provideValidTransition(): iterable + { + yield [self::buildEntityWorkflow([$u = new User()]), 'transition1', $u, 'step1']; + yield [self::buildEntityWorkflow([$u = new User()]), 'transition2', $u, 'step2']; + yield [self::buildEntityWorkflow([new User()]), 'transition2', null, 'step2']; + yield [self::buildEntityWorkflow([]), 'transition2', null, 'step2']; + yield [self::buildEntityWorkflow([new User()]), 'transition3', null, 'step3']; + yield [self::buildEntityWorkflow([]), 'transition3', null, 'step3']; + } + + public static function buildEntityWorkflow(array $futureDestUsers): EntityWorkflow + { + $registry = self::buildRegistry(null); + $baseContext = ['transition' => 'intermediate', 'transitionAt' => new \DateTimeImmutable()]; + + // test a user not is destination is blocked + $entityWorkflow = new EntityWorkflow(); + $workflow = $registry->get($entityWorkflow, 'dummy'); + $dto = new WorkflowTransitionContextDTO($entityWorkflow); + $dto->futureDestUsers = $futureDestUsers; + $workflow->apply($entityWorkflow, 'intermediate', ['context' => $dto, ...$baseContext]); + + return $entityWorkflow; + } +} diff --git a/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowGuardTransition.php b/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowGuardTransition.php new file mode 100644 index 000000000..25ed21e98 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowGuardTransition.php @@ -0,0 +1,105 @@ + [ + ['guardEntityWorkflow', 0], + ], + ]; + } + + public function guardEntityWorkflow(GuardEvent $event) + { + if (!$event->getSubject() instanceof EntityWorkflow) { + return; + } + + /** @var EntityWorkflow $entityWorkflow */ + $entityWorkflow = $event->getSubject(); + + if ($entityWorkflow->isFinal()) { + $event->addTransitionBlocker( + new TransitionBlocker( + 'workflow.The workflow is finalized', + 'd6306280-7535-11ec-a40d-1f7bee26e2c0' + ) + ); + + return; + } + + $user = $this->security->getUser(); + $metadata = $event->getWorkflow()->getMetadataStore()->getTransitionMetadata($event->getTransition()); + $systemTransitions = explode('+', $metadata['transitionGuard'] ?? 'only-dest'); + + if (null === $user) { + if (in_array('system', $systemTransitions, true)) { + // it is safe to apply this transition + return; + } + + $event->addTransitionBlocker( + new TransitionBlocker( + 'workflow.Transition is not allowed for system', + 'd9e39a18-704c-11ef-b235-8fe0619caee7' + ) + ); + + return; + } + + // for users + if (!in_array('only-dest', $systemTransitions, true)) { + $event->addTransitionBlocker( + new TransitionBlocker( + 'workflow.Only system can apply this transition', + '5b6b95e0-704d-11ef-a5a9-4b6fc11a8eeb' + ) + ); + } + + if (!$entityWorkflow->getCurrentStep()->getAllDestUser()->contains($user)) { + if ($event->getMarking()->has('initial')) { + return; + } + + $event->addTransitionBlocker(new TransitionBlocker( + 'workflow.You are not allowed to apply a transition on this workflow. Only those users are allowed: %users%', + 'f3eeb57c-7532-11ec-9495-e7942a2ac7bc', + [ + '%users%' => implode( + ', ', + $entityWorkflow->getCurrentStep()->getAllDestUser()->map(fn (User $u) => $this->userRender->renderString($u, []))->toArray() + ), + ] + )); + } + } +} diff --git a/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowTransitionEventSubscriber.php b/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowTransitionEventSubscriber.php index fe489c59d..8903d79eb 100644 --- a/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowTransitionEventSubscriber.php +++ b/src/Bundle/ChillMainBundle/Workflow/EventSubscriber/EntityWorkflowTransitionEventSubscriber.php @@ -13,20 +13,16 @@ namespace Chill\MainBundle\Workflow\EventSubscriber; use Chill\MainBundle\Entity\User; use Chill\MainBundle\Entity\Workflow\EntityWorkflow; -use Chill\MainBundle\Templating\Entity\UserRender; use Psr\Log\LoggerInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\Security\Core\Security; use Symfony\Component\Workflow\Event\Event; -use Symfony\Component\Workflow\Event\GuardEvent; -use Symfony\Component\Workflow\TransitionBlocker; final readonly class EntityWorkflowTransitionEventSubscriber implements EventSubscriberInterface { public function __construct( private LoggerInterface $chillLogger, private Security $security, - private UserRender $userRender, ) {} public static function getSubscribedEvents(): array @@ -36,48 +32,9 @@ final readonly class EntityWorkflowTransitionEventSubscriber implements EventSub 'workflow.completed' => [ ['markAsFinal', 2048], ], - 'workflow.guard' => [ - ['guardEntityWorkflow', 0], - ], ]; } - public function guardEntityWorkflow(GuardEvent $event) - { - if (!$event->getSubject() instanceof EntityWorkflow) { - return; - } - - /** @var EntityWorkflow $entityWorkflow */ - $entityWorkflow = $event->getSubject(); - - if ($entityWorkflow->isFinal()) { - $event->addTransitionBlocker( - new TransitionBlocker( - 'workflow.The workflow is finalized', - 'd6306280-7535-11ec-a40d-1f7bee26e2c0' - ) - ); - - return; - } - - if (!$entityWorkflow->getCurrentStep()->getAllDestUser()->contains($this->security->getUser())) { - if (!$event->getMarking()->has('initial')) { - $event->addTransitionBlocker(new TransitionBlocker( - 'workflow.You are not allowed to apply a transition on this workflow. Only those users are allowed: %users%', - 'f3eeb57c-7532-11ec-9495-e7942a2ac7bc', - [ - '%users%' => implode( - ', ', - $entityWorkflow->getCurrentStep()->getAllDestUser()->map(fn (User $u) => $this->userRender->renderString($u, []))->toArray() - ), - ] - )); - } - } - } - public function markAsFinal(Event $event): void { // NOTE: it is not possible to move this method to the marking store, because @@ -109,11 +66,13 @@ final readonly class EntityWorkflowTransitionEventSubscriber implements EventSub /** @var EntityWorkflow $entityWorkflow */ $entityWorkflow = $event->getSubject(); + $user = $this->security->getUser(); + $this->chillLogger->info('[workflow] apply transition on entityWorkflow', [ 'relatedEntityClass' => $entityWorkflow->getRelatedEntityClass(), 'relatedEntityId' => $entityWorkflow->getRelatedEntityId(), 'transition' => $event->getTransition()->getName(), - 'by_user' => $this->security->getUser(), + 'by_user' => $user instanceof User ? $user->getId() : (string) $user?->getUserIdentifier(), 'entityWorkflow' => $entityWorkflow->getId(), ]); } From 4e588ed0e0071b8259bd70b6733f065c4b1da8fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 11 Sep 2024 21:14:07 +0200 Subject: [PATCH 55/60] Add logging to SignatureStepStateChanger Enhanced the SignatureStepStateChanger by integrating a LoggerInterface to provide detailed logging at key points in the state transition process. This includes informational messages when marking signatures or skipping transitions, as well as debug messages when determining the next steps. --- .../Workflow/SignatureStepStateChanger.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/Bundle/ChillMainBundle/Workflow/SignatureStepStateChanger.php b/src/Bundle/ChillMainBundle/Workflow/SignatureStepStateChanger.php index efba00b02..3e2871810 100644 --- a/src/Bundle/ChillMainBundle/Workflow/SignatureStepStateChanger.php +++ b/src/Bundle/ChillMainBundle/Workflow/SignatureStepStateChanger.php @@ -15,14 +15,18 @@ use Chill\MainBundle\Entity\User; use Chill\MainBundle\Entity\Workflow\EntityWorkflowSignatureStateEnum; use Chill\MainBundle\Entity\Workflow\EntityWorkflowStep; use Chill\MainBundle\Entity\Workflow\EntityWorkflowStepSignature; +use Psr\Log\LoggerInterface; use Symfony\Component\Clock\ClockInterface; use Symfony\Component\Workflow\Registry; class SignatureStepStateChanger { + private const LOG_PREFIX = '[SignatureStepStateChanger] '; + public function __construct( private readonly Registry $registry, private readonly ClockInterface $clock, + private readonly LoggerInterface $logger, ) {} public function markSignatureAsSigned(EntityWorkflowStepSignature $signature, ?int $atIndex): void @@ -33,10 +37,16 @@ class SignatureStepStateChanger ->setStateDate($this->clock->now()) ; + $this->logger->info(self::LOG_PREFIX.'Mark signature entity as signed', ['signatureId' => $signature->getId(), 'index' => (string) $atIndex]); + if (!EntityWorkflowStepSignature::isAllSignatureNotPendingForStep($signature->getStep())) { + $this->logger->info(self::LOG_PREFIX.'This is not the last signature, skipping transition to another place', ['signatureId' => $signature->getId()]); + return; } + $this->logger->debug(self::LOG_PREFIX.'Continuing the process to find a transition', ['signatureId' => $signature->getId()]); + $entityWorkflow = $signature->getStep()->getEntityWorkflow(); $workflow = $this->registry->get($entityWorkflow, $entityWorkflow->getWorkflowName()); $metadataStore = $workflow->getMetadataStore(); @@ -54,12 +64,16 @@ class SignatureStepStateChanger } if (null === $transition) { + $this->logger->info(self::LOG_PREFIX.'The transition is not configured, will not apply a transition', ['signatureId' => $signature->getId()]); + return; } $previousUser = $this->getPreviousSender($signature->getStep()); if (null === $previousUser) { + $this->logger->info(self::LOG_PREFIX.'No previous user, will not apply a transition', ['signatureId' => $signature->getId()]); + return; } @@ -71,6 +85,8 @@ class SignatureStepStateChanger 'transitionAt' => $this->clock->now(), 'transition' => $transition, ]); + + $this->logger->info(self::LOG_PREFIX.'Transition automatically applied', ['signatureId' => $signature->getId()]); } private function getPreviousSender(EntityWorkflowStep $entityWorkflowStep): ?User From f1505a9d15390974caf6fc64f5ee84c8ac5d9272 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 11 Sep 2024 21:16:54 +0200 Subject: [PATCH 56/60] Add locale to workflow URLs in notification templates This is required to send notification within a console command --- ...low_notification_on_transition_completed_content.fr.txt.twig | 2 +- .../views/Workflow/workflow_send_access_key.fr.txt.twig | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/workflow_notification_on_transition_completed_content.fr.txt.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/workflow_notification_on_transition_completed_content.fr.txt.twig index 1e8469968..034b9c38a 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/workflow_notification_on_transition_completed_content.fr.txt.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/workflow_notification_on_transition_completed_content.fr.txt.twig @@ -8,6 +8,6 @@ Vous êtes invités à valider cette étape au plus tôt. Vous pouvez visualiser le workflow sur cette page: -{{ absolute_url(path('chill_main_workflow_show', {'id': entity_workflow.id})) }} +{{ absolute_url(path('chill_main_workflow_show', {'id': entity_workflow.id, '_locale': 'fr'})) }} Cordialement, diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/workflow_send_access_key.fr.txt.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/workflow_send_access_key.fr.txt.twig index 6237cb68a..b4574567d 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/workflow_send_access_key.fr.txt.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/workflow_send_access_key.fr.txt.twig @@ -6,7 +6,7 @@ Titre du workflow: "{{ entityTitle }}". Vous êtes invité·e à valider cette étape. Pour obtenir un accès, vous pouvez cliquer sur le lien suivant: -{{ absolute_url(path('chill_main_workflow_grant_access_by_key', {'id': entity_workflow.currentStep.id, 'accessKey': entity_workflow.currentStep.accessKey})) }} +{{ absolute_url(path('chill_main_workflow_grant_access_by_key', {'id': entity_workflow.currentStep.id, 'accessKey': entity_workflow.currentStep.accessKey, '_locale': fr})) }} Dès que vous aurez cliqué une fois sur le lien, vous serez autorisé à valider cette étape. From 18af2ca70b3ef76b7047e9275c296b1e25c05230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 11 Sep 2024 21:52:34 +0200 Subject: [PATCH 57/60] Handle null transitionBy and improve display logic Added checks for null transitionBy cases in workflow templates to display "Automated transition" when applicable. Also improved conditional rendering for 'destUser' and 'ccUser' fields to avoid empty elements. --- .../views/Workflow/_history.html.twig | 8 +++-- .../views/Workflow/macro_breadcrumb.html.twig | 30 +++++++++++-------- .../translations/messages.fr.yml | 1 + 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_history.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_history.html.twig index a56656102..670d27ecc 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/_history.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/_history.html.twig @@ -40,11 +40,13 @@ {% set forward = workflow_metadata(step.entityWorkflow, 'isForward', transition) %}
      - {% if step.transitionBy is not null %}
      - {{ step.transitionBy|chill_entity_render_box({'at_date': step.transitionAt}) }} + {%- if step.transitionBy is not null -%} + {{ step.transitionBy|chill_entity_render_box({'at_date': step.transitionAt}) }} + {% else %} + {{ 'workflow.Automated transition'|trans }} + {%- endif -%}
      - {% endif %}
      {{ step.transitionAt|format_datetime('long', 'medium') }}
      diff --git a/src/Bundle/ChillMainBundle/Resources/views/Workflow/macro_breadcrumb.html.twig b/src/Bundle/ChillMainBundle/Resources/views/Workflow/macro_breadcrumb.html.twig index 71d1efcf1..e3087769f 100644 --- a/src/Bundle/ChillMainBundle/Resources/views/Workflow/macro_breadcrumb.html.twig +++ b/src/Bundle/ChillMainBundle/Resources/views/Workflow/macro_breadcrumb.html.twig @@ -3,24 +3,28 @@ {% if step.previous is not null %}
    • {{ 'By'|trans ~ ' : ' }} - {{ step.previous.transitionBy|chill_entity_render_box({'at_date': step.previous.transitionAt }) }} + {% if step.previous.transitionBy is not null %}{{ step.previous.transitionBy|chill_entity_render_box({'at_date': step.previous.transitionAt }) }}{% else %}{{ 'workflow.Automated transition'|trans }}{% endif %}
    • {{ 'Le'|trans ~ ' : ' }} {{ step.previous.transitionAt|format_datetime('short', 'short') }}
      • -
    • - {{ 'workflow.For'|trans ~ ' : ' }} - - {% for d in step.destUser %}{{ d|chill_entity_render_string({'at_date': step.previous.transitionAt}) }}{% if not loop.last %}, {% endif %}{% endfor %} - -
      • -
    • - {{ 'workflow.Cc'|trans ~ ' : ' }} - - {% for u in step.ccUser %}{{ u|chill_entity_render_string({'at_date': step.previous.transitionAt }) }}{% if not loop.last %}, {% endif %}{% endfor %} - -
      • + {% if step.destUser|length > 0 %} +
    • + {{ 'workflow.For'|trans ~ ' : ' }} + + {% for d in step.destUser %}{{ d|chill_entity_render_string({'at_date': step.previous.transitionAt}) }}{% if not loop.last %}, {% endif %}{% endfor %} + +
      • + {% endif %} + {% if step.ccUser|length > 0 %} +
    • + {{ 'workflow.Cc'|trans ~ ' : ' }} + + {% for u in step.ccUser %}{{ u|chill_entity_render_string({'at_date': step.previous.transitionAt }) }}{% if not loop.last %}, {% endif %}{% endfor %} + +
      • + {% endif %} {% else %}
    • {{ 'workflow.Created by'|trans ~ ' : ' }} diff --git a/src/Bundle/ChillMainBundle/translations/messages.fr.yml b/src/Bundle/ChillMainBundle/translations/messages.fr.yml index f5c2cfc46..0c0fce51c 100644 --- a/src/Bundle/ChillMainBundle/translations/messages.fr.yml +++ b/src/Bundle/ChillMainBundle/translations/messages.fr.yml @@ -530,6 +530,7 @@ workflow: Put on hold: Mettre en attente Remove hold: Enlever la mise en attente On hold: En attente + Automated transition: Transition automatique signature_zone: title: Appliquer les signatures électroniques From c23568032c32a2ae7ff6de6cd6dbc33a73b91a45 Mon Sep 17 00:00:00 2001 From: nobohan Date: Wed, 4 Sep 2024 10:14:08 +0200 Subject: [PATCH 58/60] Signature: add a signature zone manually --- .../Resources/public/types.ts | 4 +- .../public/vuejs/DocumentSignature/App.vue | 163 +++++++++++++----- .../public/vuejs/DocumentSignature/index.ts | 4 +- 3 files changed, 128 insertions(+), 43 deletions(-) diff --git a/src/Bundle/ChillDocStoreBundle/Resources/public/types.ts b/src/Bundle/ChillDocStoreBundle/Resources/public/types.ts index 840421991..921da3a9a 100644 --- a/src/Bundle/ChillDocStoreBundle/Resources/public/types.ts +++ b/src/Bundle/ChillDocStoreBundle/Resources/public/types.ts @@ -83,7 +83,7 @@ export interface PDFPage { height: number, } export interface SignatureZone { - index: number, + index: number | null, x: number, y: number, width: number, @@ -98,3 +98,5 @@ export interface Signature { } export type SignedState = 'pending' | 'signed' | 'rejected' | 'canceled' | 'error'; + +export type CanvasEvent = 'select' | 'add'; diff --git a/src/Bundle/ChillDocStoreBundle/Resources/public/vuejs/DocumentSignature/App.vue b/src/Bundle/ChillDocStoreBundle/Resources/public/vuejs/DocumentSignature/App.vue index 47edd87c3..04cdf9eea 100644 --- a/src/Bundle/ChillDocStoreBundle/Resources/public/vuejs/DocumentSignature/App.vue +++ b/src/Bundle/ChillDocStoreBundle/Resources/public/vuejs/DocumentSignature/App.vue @@ -80,8 +80,32 @@
      • + +
      +
      +
      + +
      +
      + +
      +
      +
      +
      -
      +
      -
      +
      -
      -
      - -
      -
      - -
      -
      +
      +
      +
      +
      + +
      +
      + +
      -
      +
      -
      - +
      @@ -510,7 +503,7 @@ downloadAndOpen();