[BO - Notifications] Refactorisation envoi de mail #1134

sfinx13 · 2023-04-03T16:33:52Z

Ticket

Description

L'approche actuelle rend le code difficile à maintenir et à étendre car elle est très rigide et ne permet pas d'étendre la fonctionnalité d'envoi de base.
L'idée est de créer une hiérarchie de classes ou chaque classe correspond à un type de notification qui étend une classe principale qui encapsule la logique d'envoi.

Cela permet d'avoir une meilleur visibilité sur les différents mail envoyés et de rendre le code plus flexible, plus facile à maintenir et à étendre. On pourrait imaginer apporter des fonctionnalités supplémentaires sur un type de mail particulier.

Changements apportés

Remplacement de l'ancien service NotificationService par NotificationMailerRegistry qui est un registre qui contient l'ensemble des maiil de notification référencé par le tag symfony app.notification_mailer
Remplacement du tableau associatif par une classe NotificationMail pour encapsuler les données utile à l'envoi

        private readonly NotificationMailerType $type,
        private readonly array|string $to,
        private readonly ?string $fromEmail = null,
        private readonly ?string $fromFullname = null,
        private readonly ?string $message = null,
        private readonly ?Territory $territory = null,
        private readonly ?User $user = null,
        private readonly ?Signalement $signalement = null,
        private readonly ?\Throwable $event = null,
        private readonly mixed $attachment = null,
        private readonly ?string $motif = null,
        private readonly ?string $cronLabel = null,
        private readonly ?int $cronCount = null,
        private readonly array $params = [],

Définition des type d'email dans une énumération NotificationMailerType
Implémentation d'une classe abstraite qui va encapsuler la logique d'envoi AbstractNotificationMailer
Implémentation des 23 classes enfants héritant de AbstractNotificationMailer réparti selon une arborescence métier qui peut évoluer, aucune contrainte existe.

.
└── Mailer/
    ├── Mail/
    │   ├── Account/
    │   │   ├── AccountActivationNotificationMailer.php
    │   │   └── ...
    │   ├── Contact/
    │   │   ├── ContactFormMailer.php
    │   │   └── ....
    │   ├── Cron/
    │   │   ├── CronMailer.php
    │   │   └── ...
    │   ├── Error/
    │   │   ├── ErrorSignalementMailer.php
    │   │   └── ...
    │   ├── Signalement/
    │   │   ├── SignalementNewMailer.php
    │   │   └── ...
    │   ├── Suivi/
    │   │   ├── SuiviNewCommentBackMailer.php
    │   │   └── ....
    │   ├── AbstractNotificationMailer.php
    │   └── NotificationMailerInterface.php
    ├── NotificationMail.php
    ├── NotificationMailerRegistry.php
    └── NotificationMailerType.php

Comment envoyer un mail ?

Définir une constante dans NotificationMailerType.php pour identifier un email

enum NotificationMailerType
{
    ....
    case TYPE_INTERVENTION_NEW_DATE_VISITE_USAGER;
}

Créer une classe Mailer pour configurer un email

namespace App\Service\Mailer\Mail\Intervention;

// ....

class InterventionNewDateVisiteUsagerMailer extends AbstractNotificationMailer
{
    protected ?NotificationMailerType $mailerType = NotificationMailerType::TYPE_INTERVENTION_NEW_DATE_VISITE_USAGER;
    protected ?string $mailerSubject = 'Une visite de votre logement est prévue';
    protected ?string $mailerButtonText = 'Accéder à mon signalement';
    protected ?string $mailerTemplate = 'new_intervention_date_visite_usager_email';

    public function __construct(
        protected MailerInterface $mailer,
        protected ParameterBagInterface $parameterBag,
        protected LoggerInterface $logger
    ) {
        parent::__construct($this->mailer, $this->parameterBag, $this->logger);
    }
}

    public function getMailerParamsFromNotification(NotificationMail $notificationMail): array
    {
        $signalement = $notificationMail->getSignalement();
        return [
            'ref_signalement' => $signalement->getReference(),
        ];
    }

Envoyer un email

Injecter le service NotificationMailerRegistry et définir un objet Notification

#[Route('/edit-date-visite', name: 'edit_date_visite')]
    public function nomAction(
        NotificationMailerRegistry $notificationMailerRegistry,
        Signalement $signalement,
        Request $request
    ): Response {
                $notificationMailerRegistry->send(
                    new NotificationMail(
                        type: NotificationMailerType::TYPE_INTERVENTION_NEW_DATE_VISITE_USAGER,
                        to: 'john.doe@yopmail.com',
                        territory: $this->getUser()->getTerritory(),
                        signalement: $signalement
                    )
                );
        }
    }

Tests (22 emails)

Tester l'ensemble des cas métiers qui déclenche les 23 mails.

github-advanced-security · 2023-04-03T16:34:53Z

You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

sfinx13 · 2023-04-05T17:04:51Z

src/Command/AskFeedbackUsagerCommand.php

+                NotificationMailerType::TYPE_CRON,
+                $this->parameterBag->get('admin_email'),
+                [
+                    'cron_label' => 'demande de feedback à l\'usager',


Plus besoin de passer l'url, elle est passé en tant que paramètre par défaut

sfinx13 · 2023-04-05T17:37:18Z

templates/emails/accuse_reception_email.html.twig

@@ -23,7 +23,7 @@
                    <tbody>
                    <tr style="text-align: center;width: 100%">
                        <td>
-                            <a href="{{ lien_suivi|raw }}" target="_blank" rel="noopener">{{ btnText|capitalize }}</a></td>
+                            <a href="{{ lien_suivi|raw }}" target="_blank" rel="noopener">{{ btntext|capitalize }}</a></td>


Il y'avait les deux variables btnText et btntext j'ai gardé celui ou il y'avais moins de fichier à modifier

src/Service/Mailer/Mail/Signalement/AssignementNewMailer.php

src/Service/Mailer/Mail/Signalement/SignalementClosedToOnePartnerMailer.php

src/Service/Mailer/Mail/Signalement/SignalementNewMailer.php

src/Service/Mailer/Mail/Error/ErrorSignalementMailer.php

src/Service/Mailer/Mail/Account/AccountLostPassword.php

src/Manager/UserManager.php

src/Service/Mailer/Mail/Account/AccountReactivationMailer.php

src/Service/Mailer/Mail/Account/AccountDeleteMailer.php

sfinx13 · 2023-04-07T11:13:43Z

src/Controller/Back/BackSignalementActionController.php

@@ -34,36 +41,27 @@ public function validationResponseSignalement(Signalement $signalement, Request
                $signalement->setCodeSuivi(md5(uniqid()));
                $toRecipients = $signalement->getMailUsagers();
                foreach ($toRecipients as $toRecipient) {
-                    $notificationService->send(


Avant @emilschn

sfinx13 · 2023-04-07T11:26:37Z

src/Controller/Back/BackSignalementActionController.php

-                            ),
-                        ],
-                        $signalement->getTerritory()
+                    $notificationMailerRegistry->send(


Apres @emilschn la construction des paramètres se fait dans la classe dorénavant dans la majorité des cas, il reste toujours possible de passer des paramètres.

src/Service/Mailer/NotificationMail.php

src/Service/Mailer/Mail/Account/AccountTransferMailer.php

hmeneuvrier

J'ai refait une passe de tests, j'ai mis mes commentaires

sfinx13 · 2023-04-07T15:37:48Z

config/packages/security.yaml

@@ -36,7 +36,7 @@ security:
                signature_properties: [id, email]
                default_target_path: login_creation_pass
                failure_path: login_activation_fail
-                max_uses: 1
+                max_uses: 3


Aujourd'hui deux systèmes de reset-password cohabite, mise à jour de la configuration afin de permettre aux utilisateurs de cliquer au max 3 fois sur le lien.
Pour info LoginLink est utilisé de maniere détourné car cela sert à faire de la connexion sans mot de passe et non à générer des liens de reset-password. Doit être remplacer par le système qui a été mis en place coté BO

sfinx13 · 2023-04-07T15:40:27Z

src/Command/GetGeolocCommand.php

@@ -1,91 +0,0 @@
-<?php
-


Jamais utilisé depuis 8 mois

sfinx13 · 2023-04-07T15:42:49Z

src/Command/RemindInactiveUserCommand.php

+                    territory: $user->getTerritory(),
+                    user: $user,
+                    params: [
+                        'nb_signalements' => $userItem['nb_signalements'],


Seulement 2 mails ont un tableau associatif passé en paramètre celui ci et celui d'ESABORA

sfinx13 · 2023-04-07T15:50:26Z

src/Repository/AffectationRepository.php

@@ -63,63 +58,6 @@ public function countByStatusForUser($user, Territory|null $territory, Qualifica
            ->getResult();
    }

-    public function findByStatusAndOrCityForUser(User|UserInterface|null $user, array $options, int|null $export): Paginator|array


Plus utilisé depuis la livraison de la 1.6.0

sfinx13 · 2023-04-07T15:52:55Z

src/Service/Mailer/Mail/AbstractNotificationMailer.php

+    {
+    }
+
+    abstract public function getMailerParamsFromNotification(NotificationMail $notificationMail): array;


Il est obligatoire d'implementer getMailerParamsFromNotification même si vous n'avez pas de paramètre à utiliser ce qui est assez rare dans ce cas la, passez un tableau vide comme dans AccountDeleteMailer

sfinx13 · 2023-04-07T15:54:02Z

src/Service/Mailer/Mail/AbstractNotificationMailer.php

+        return $this->parameterBag->get('host_url').$this->urlGenerator->generate($route, $params);
+    }
+
+    public function updateMailerSubjectFromNotification(NotificationMail $notificationMail): void


Si vous devez avoir un objet avec des paramètres implémenter updateMailerSubjectFromNotification

sfinx13 · 2023-04-07T15:55:59Z

src/Service/Mailer/Mail/Signalement/SignalementClosedToAllPartnersMailer.php

+        ];
+    }
+
+    public function updateMailerSubjectFromNotification(NotificationMail $notificationMail): void


Pour gérer les placeholder dans un objet de mail

sfinx13 · 2023-04-07T15:58:49Z

src/Service/Mailer/NotificationMail.php

+
+class NotificationMail
+{
+    public function __construct(


Porte les paramètres, peut être amené à évoluer si ces paramètre ne permettent pas de récupérer l'info dont on a besoin

emilschn · 2023-04-11T08:52:14Z

Pour moi, c'est ok sur les fonctionnalités re-testées.

hmeneuvrier · 2023-04-11T13:14:45Z

ok pour moi aussi !

sonarcloud · 2023-04-11T13:46:56Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

No Coverage information
2.6% Duplication

sfinx13 changed the title ~~Move notificationService in Mailer folder~~ [BO - Notifications] Revoir les notifications + architecture envoi de mail Apr 3, 2023

sfinx13 changed the title ~~[BO - Notifications] Revoir les notifications + architecture envoi de mail~~ [BO - Notifications] Revoir les notifications + refactorisation envoi de mail Apr 4, 2023

sfinx13 changed the title ~~[BO - Notifications] Revoir les notifications + refactorisation envoi de mail~~ [BO - Notifications] Refactorisation envoi de mail Apr 4, 2023

sfinx13 commented Apr 5, 2023

View reviewed changes

sfinx13 requested review from hmeneuvrier and emilschn April 6, 2023 11:49

sfinx13 marked this pull request as ready for review April 6, 2023 11:49

hmeneuvrier reviewed Apr 6, 2023

View reviewed changes

sfinx13 commented Apr 7, 2023

View reviewed changes

hmeneuvrier reviewed Apr 7, 2023

View reviewed changes

src/Service/Mailer/NotificationMail.php Show resolved Hide resolved

hmeneuvrier reviewed Apr 7, 2023

View reviewed changes

src/Service/Mailer/Mail/Account/AccountTransferMailer.php Show resolved Hide resolved

hmeneuvrier reviewed Apr 7, 2023

View reviewed changes

sfinx13 commented Apr 7, 2023

View reviewed changes

emilschn approved these changes Apr 11, 2023

View reviewed changes

sfinx13 added 2 commits April 11, 2023 15:11

Move notificationService in Mailer folder

1ebee3b

Move notificationService in Mailer folder

67381ed

refaco sending email system #853

3a67ebc

hmeneuvrier approved these changes Apr 11, 2023

View reviewed changes

sfinx13 added 8 commits April 11, 2023 15:15

refaco sending email system #853

b547e98

add class mailer #853

9d7c874

remove some legacy code #853

5f71b06

remove some legacy code #853

3412b55

apply some refacto #853

807fb42

apply some refacto #853

e9df111

remove duplicate parameter #853

25f5ce9

rename Notification class to NotificationMail

70afcf3

sfinx13 added 6 commits April 11, 2023 15:19

Parameter managed by child class

cfc09e8

Parameter managed by child class

19a42f5

apply some fixes

e487aa8

apply some fixes

1f43dca

apply some fixes

9785d63

add eof yaml

3402bc5

sfinx13 force-pushed the feature/853-review-notifications branch from 8451018 to 3402bc5 Compare April 11, 2023 13:44

hmeneuvrier merged commit 58434bf into develop Apr 11, 2023

hmeneuvrier deleted the feature/853-review-notifications branch April 11, 2023 13:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BO - Notifications] Refactorisation envoi de mail #1134

[BO - Notifications] Refactorisation envoi de mail #1134

sfinx13 commented Apr 3, 2023 •

edited

Loading

github-advanced-security bot commented Apr 3, 2023

sfinx13 Apr 5, 2023

sfinx13 Apr 5, 2023

sfinx13 Apr 7, 2023

sfinx13 Apr 7, 2023

hmeneuvrier left a comment

sfinx13 Apr 7, 2023

sfinx13 Apr 7, 2023

sfinx13 Apr 7, 2023

sfinx13 Apr 7, 2023

sfinx13 Apr 7, 2023

sfinx13 Apr 7, 2023 •

edited

Loading

sfinx13 Apr 7, 2023 •

edited

Loading

sfinx13 Apr 7, 2023 •

edited

Loading

emilschn commented Apr 11, 2023

hmeneuvrier commented Apr 11, 2023

sonarcloud bot commented Apr 11, 2023

[BO - Notifications] Refactorisation envoi de mail #1134

[BO - Notifications] Refactorisation envoi de mail #1134

Conversation

sfinx13 commented Apr 3, 2023 • edited Loading

Ticket

Description

Changements apportés

Comment envoyer un mail ?

Définir une constante dans NotificationMailerType.php pour identifier un email

Créer une classe Mailer pour configurer un email

Envoyer un email

Tests (22 emails)

github-advanced-security bot commented Apr 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hmeneuvrier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfinx13 Apr 7, 2023 • edited Loading

Choose a reason for hiding this comment

sfinx13 Apr 7, 2023 • edited Loading

Choose a reason for hiding this comment

sfinx13 Apr 7, 2023 • edited Loading

Choose a reason for hiding this comment

emilschn commented Apr 11, 2023

hmeneuvrier commented Apr 11, 2023

sonarcloud bot commented Apr 11, 2023

sfinx13 commented Apr 3, 2023 •

edited

Loading

sfinx13 Apr 7, 2023 •

edited

Loading

sfinx13 Apr 7, 2023 •

edited

Loading

sfinx13 Apr 7, 2023 •

edited

Loading