From cb1ea8c622afffc180bb5edb3669e9940ef76849 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 19 Apr 2023 12:04:52 +0200 Subject: [PATCH 1/4] DX: [mailer] deprecate the chill mailer based on swift mailer --- composer.json | 1 - .../ChillMainBundle/Notification/Mailer.php | 69 +++++++------------ .../PasswordRecover/RecoverPasswordHelper.php | 68 ++++++------------ .../config/services/notification.yaml | 6 -- .../config/services/security.yaml | 6 +- 5 files changed, 47 insertions(+), 103 deletions(-) diff --git a/composer.json b/composer.json index 033b3f19b..b47856954 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,6 @@ "symfony/monolog-bundle": "^3.5", "symfony/security-bundle": "^4.4", "symfony/serializer": "^5.3", - "symfony/swiftmailer-bundle": "^3.5", "symfony/templating": "^4.4", "symfony/translation": "^4.4", "symfony/twig-bundle": "^4.4", diff --git a/src/Bundle/ChillMainBundle/Notification/Mailer.php b/src/Bundle/ChillMainBundle/Notification/Mailer.php index c63a96f04..307d5d1df 100644 --- a/src/Bundle/ChillMainBundle/Notification/Mailer.php +++ b/src/Bundle/ChillMainBundle/Notification/Mailer.php @@ -15,7 +15,10 @@ use Chill\MainBundle\Entity\User; use Psr\Log\LoggerInterface; use Swift_Mailer; use Swift_Message; +use Symfony\Component\Mailer\MailerInterface; +use Symfony\Component\Mime\Email; use Symfony\Component\Routing\RouterInterface; +use Symfony\Component\Templating\EngineInterface; use Symfony\Contracts\Translation\TranslatorInterface; use Twig\Environment; @@ -26,43 +29,34 @@ use function call_user_func; * Classe d'aide pour l'envoi de notification. * * Héberge toutes les méthodes pour ré-écrire les URL en fonction de la langue de l'utilisateur. + * + * @deprecated use the MailerInterface */ class Mailer { - /** - * @var Swift_Mailer - */ - protected $forcedMailer; - /** * @var LoggerInterface */ - protected $logger; - - /** - * @var Swift_Mailer - */ - protected $mailer; + private $logger; /** * @var array */ - protected $routeParameters; + private $routeParameters; /** * @var RouterInterface */ - protected $router; + private $router; /** * @var TranslatorInterface */ - protected $translator; + private $translator; - /** - * @var \Twig\Environment - */ - protected $twig; + private EngineInterface $twig; + + private MailerInterface $mailer; /** * Mailer constructor. @@ -70,11 +64,9 @@ class Mailer * @param $routeParameters */ public function __construct( + MailerInterface $mailer, LoggerInterface $logger, - Environment $twig, - Swift_Mailer $mailer, - // due to bug https://github.com/symfony/swiftmailer-bundle/issues/127 - // \Swift_Transport $mailerTransporter, + EngineInterface $twig, RouterInterface $router, TranslatorInterface $translator, $routeParameters @@ -82,7 +74,6 @@ class Mailer $this->logger = $logger; $this->twig = $twig; $this->mailer = $mailer; - //$this->forcedMailer = new \Swift_Mailer($mailerTransporter); $this->router = $router; $this->translator = $translator; $this->routeParameters = $routeParameters; @@ -115,20 +106,6 @@ class Mailer return $content; } - /** - * @param $force - * - * @throws \Symfony\Component\Mailer\Exception\TransportExceptionInterface - */ - public function sendMessage(Swift_Message $message, $force) - { - if ($force) { - $this->forcedMailer->send($message); - } else { - $this->mailer->send($message); - } - } - /** * Envoie une notification à un utilisateur. * @@ -155,23 +132,25 @@ class Mailer $subject[2] ?? null ); - $message = (new Swift_Message($subjectI18n)) - ->setFrom($fromEmail, $fromName) - ->setTo($to); + $email = new Email(); + $email->addTo($to)->subject($subjectI18n); foreach ($bodies as $contentType => $content) { - $message->setBody($content, $contentType); + match ($contentType) { + 'text/plain' => $email->text($content), + default => $email->text($content), + }; } if (null !== $callback) { - call_user_func($callback, $message); + call_user_func($callback, $email); } $this->logger->info('[notification] Sending notification', [ - 'to' => $message->getTo(), - 'subject' => $message->getSubject(), + 'to' => $email->getTo(), + 'subject' => $email->getSubject() ]); - $this->sendMessage($message, $force); + $this->mailer->send($email); } } diff --git a/src/Bundle/ChillMainBundle/Security/PasswordRecover/RecoverPasswordHelper.php b/src/Bundle/ChillMainBundle/Security/PasswordRecover/RecoverPasswordHelper.php index 07114e4b5..42881e291 100644 --- a/src/Bundle/ChillMainBundle/Security/PasswordRecover/RecoverPasswordHelper.php +++ b/src/Bundle/ChillMainBundle/Security/PasswordRecover/RecoverPasswordHelper.php @@ -14,6 +14,8 @@ namespace Chill\MainBundle\Security\PasswordRecover; use Chill\MainBundle\Entity\User; use Chill\MainBundle\Notification\Mailer; use DateTimeInterface; +use Symfony\Bridge\Twig\Mime\TemplatedEmail; +use Symfony\Component\Mailer\MailerInterface; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use function array_merge; @@ -22,33 +24,26 @@ class RecoverPasswordHelper { public const RECOVER_PASSWORD_ROUTE = 'password_recover'; - /** - * @var Mailer - */ - protected $mailer; - - protected $routeParameters; + private MailerInterface $mailer; /** * @var TokenManager */ - protected $tokenManager; + private $tokenManager; /** * @var UrlGeneratorInterface */ - protected $urlGenerator; + private $urlGenerator; public function __construct( TokenManager $tokenManager, UrlGeneratorInterface $urlGenerator, - Mailer $mailer, - array $routeParameters + MailerInterface $mailer, ) { $this->tokenManager = $tokenManager; $this->urlGenerator = $urlGenerator; $this->mailer = $mailer; - $this->routeParameters = $routeParameters; } /** @@ -59,27 +54,14 @@ class RecoverPasswordHelper */ public function generateUrl(User $user, DateTimeInterface $expiration, $absolute = true, array $parameters = []) { - $context = $this->urlGenerator->getContext(); - $previousHost = $context->getHost(); - $previousScheme = $context->getScheme(); - - $context->setHost($this->routeParameters['host']); - $context->setScheme($this->routeParameters['scheme']); - - $url = $this->urlGenerator->generate( + return $this->urlGenerator->generate( self::RECOVER_PASSWORD_ROUTE, array_merge( $this->tokenManager->generate($user, $expiration), $parameters ), - $absolute ? UrlGeneratorInterface::ABSOLUTE_URL : UrlGeneratorInterface::ABSOLUTE_PATH + UrlGeneratorInterface::ABSOLUTE_URL ); - - // reset the host - $context->setHost($previousHost); - $context->setScheme($previousScheme); - - return $url; } public function sendRecoverEmail( @@ -91,26 +73,20 @@ class RecoverPasswordHelper array $additionalUrlParameters = [], $emailSubject = 'Recover your password' ) { - $content = $this->mailer->renderContentToUser( - $user, - $template, - array_merge( - [ - 'user' => $user, - 'url' => $this->generateUrl($user, $expiration, true, $additionalUrlParameters), - ], - $templateParameters - ) - ); + if (null === $user->getEmail() || '' === trim($user->getEmail())) { + throw new \UnexpectedValueException("No emaail associated to the user"); + } - $this->mailer->sendNotification( - $user, - [$emailSubject], - [ - 'text/plain' => $content, - ], - null, - $force - ); + $email = (new TemplatedEmail()) + ->subject($emailSubject) + ->to($user->getEmail()) + ->textTemplate($template) + ->context([ + 'user' => $user, + 'url' => $this->generateUrl($user, $expiration, true, $additionalUrlParameters), + ...$templateParameters + ]); + + $this->mailer->send($email); } } diff --git a/src/Bundle/ChillMainBundle/config/services/notification.yaml b/src/Bundle/ChillMainBundle/config/services/notification.yaml index bb0b6bb8c..29cbce946 100644 --- a/src/Bundle/ChillMainBundle/config/services/notification.yaml +++ b/src/Bundle/ChillMainBundle/config/services/notification.yaml @@ -10,12 +10,6 @@ services: Chill\MainBundle\Notification\Mailer: arguments: - $logger: '@Psr\Log\LoggerInterface' - $twig: '@Twig\Environment' - $mailer: '@swiftmailer.mailer.default' - # $mailerTransporter: '@swiftmailer.transport' - $router: '@Symfony\Component\Routing\RouterInterface' - $translator: '@Symfony\Contracts\Translation\TranslatorInterface' $routeParameters: '%chill_main.notifications%' Chill\MainBundle\Notification\NotificationHandlerManager: diff --git a/src/Bundle/ChillMainBundle/config/services/security.yaml b/src/Bundle/ChillMainBundle/config/services/security.yaml index c0d9f98b9..38089becf 100644 --- a/src/Bundle/ChillMainBundle/config/services/security.yaml +++ b/src/Bundle/ChillMainBundle/config/services/security.yaml @@ -61,11 +61,7 @@ services: arguments: $secret: '%kernel.secret%' - Chill\MainBundle\Security\PasswordRecover\RecoverPasswordHelper: - arguments: - $tokenManager: '@Chill\MainBundle\Security\PasswordRecover\TokenManager' - $mailer: '@Chill\MainBundle\Notification\Mailer' - $routeParameters: "%chill_main.notifications%" + Chill\MainBundle\Security\PasswordRecover\RecoverPasswordHelper: ~ Chill\MainBundle\Security\PasswordRecover\PasswordRecoverEventSubscriber: arguments: From a1421ea99f2c502f46f3ffbf93a73eb0e9963aa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 19 Apr 2023 12:58:40 +0200 Subject: [PATCH 2/4] Fix: fix usage of scope picker with unactives scopes --- .../Form/Type/ScopePickerType.php | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Form/Type/ScopePickerType.php b/src/Bundle/ChillMainBundle/Form/Type/ScopePickerType.php index 79ed6df40..6f1a5837f 100644 --- a/src/Bundle/ChillMainBundle/Form/Type/ScopePickerType.php +++ b/src/Bundle/ChillMainBundle/Form/Type/ScopePickerType.php @@ -60,16 +60,17 @@ class ScopePickerType extends AbstractType public function buildForm(FormBuilderInterface $builder, array $options) { - $items = array_filter( - $this->authorizationHelper->getReachableScopes( - $this->security->getUser(), - $options['role'] instanceof Role ? $options['role']->getRole() : $options['role'], - $options['center'] - ), - static function (Scope $s) { - return $s->isActive(); - } - ); + $items = array_values( + array_filter( + $this->authorizationHelper->getReachableScopes( + $this->security->getUser(), + $options['role'] instanceof Role ? $options['role']->getRole() : $options['role'], + $options['center'] + ), + static function (Scope $s) { + return $s->isActive(); + } + )); if (0 === count($items)) { throw new RuntimeException('no scopes are reachable. This form should not be shown to user'); From 906d1fdab51bdc17c1c15d6901ebe09b9a0eee06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 19 Apr 2023 13:01:48 +0200 Subject: [PATCH 3/4] DX: remove usage of deprecated Role class --- .../Controller/SingleTaskController.php | 13 +++++-------- src/Bundle/ChillTaskBundle/Form/SingleTaskType.php | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Bundle/ChillTaskBundle/Controller/SingleTaskController.php b/src/Bundle/ChillTaskBundle/Controller/SingleTaskController.php index 530f54b01..0cdcd7173 100644 --- a/src/Bundle/ChillTaskBundle/Controller/SingleTaskController.php +++ b/src/Bundle/ChillTaskBundle/Controller/SingleTaskController.php @@ -33,6 +33,7 @@ use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\Form\Extension\Core\Type\SubmitType; use Symfony\Component\Form\FormFactoryInterface; +use Symfony\Component\Form\FormInterface; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -205,7 +206,7 @@ final class SingleTaskController extends AbstractController . 'allowed to edit this task'); $event = (new UIEvent('single-task', $task)) - ->setForm($this->setCreateForm($task, new Role(TaskVoter::UPDATE))); + ->setForm($this->setCreateForm($task, TaskVoter::UPDATE)); $this->eventDispatcher->dispatch(UIEvent::EDIT_FORM, $event); $form = $event->getForm(); @@ -557,7 +558,7 @@ final class SingleTaskController extends AbstractController $this->denyAccessUnlessGranted($role, $task, 'You are not ' . 'allowed to create this task'); - $form = $this->setCreateForm($task, new Role($role)); + $form = $this->setCreateForm($task, $role); $form->handleRequest($request); @@ -650,7 +651,7 @@ final class SingleTaskController extends AbstractController /** * @return \Symfony\Component\Form\FormInterface */ - protected function setCreateForm(SingleTask $task, Role $role) + protected function setCreateForm(SingleTask $task, string $role) { $form = $this->createForm(SingleTaskType::class, $task, [ 'role' => $role, @@ -684,12 +685,8 @@ final class SingleTaskController extends AbstractController /** * Creates a form to delete a Task entity by id. - * - * @param mixed $id The entity id - * - * @return \Symfony\Component\Form\Form The form */ - private function createDeleteForm($id) + private function createDeleteForm($id): FormInterface { return $this->createFormBuilder() ->setAction($this->generateUrl( diff --git a/src/Bundle/ChillTaskBundle/Form/SingleTaskType.php b/src/Bundle/ChillTaskBundle/Form/SingleTaskType.php index 56e33954b..d27a2aa06 100644 --- a/src/Bundle/ChillTaskBundle/Form/SingleTaskType.php +++ b/src/Bundle/ChillTaskBundle/Form/SingleTaskType.php @@ -81,7 +81,7 @@ class SingleTaskType extends AbstractType ->add('circle', ScopePickerType::class, [ 'center' => $center, 'role' => $options['role'], - 'required' => false, + 'required' => true, ]); } } From e6163b2bc3a8e1d7bb4581586ab74321c5b1b37a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 19 Apr 2023 13:05:21 +0200 Subject: [PATCH 4/4] DX: fix cs --- .../DocGenerator/ListActivitiesByAccompanyingPeriodContext.php | 1 - .../Controller/AddressToReferenceMatcherController.php | 1 - src/Bundle/ChillMainBundle/Form/Type/ScopePickerType.php | 3 ++- src/Bundle/ChillMainBundle/Form/WorkflowStepType.php | 2 +- .../ChillMainBundle/migrations/Version20230306145728.php | 1 - src/Bundle/ChillTaskBundle/Controller/SingleTaskController.php | 1 + 6 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Bundle/ChillActivityBundle/Service/DocGenerator/ListActivitiesByAccompanyingPeriodContext.php b/src/Bundle/ChillActivityBundle/Service/DocGenerator/ListActivitiesByAccompanyingPeriodContext.php index 266fadfee..b2272dd7b 100644 --- a/src/Bundle/ChillActivityBundle/Service/DocGenerator/ListActivitiesByAccompanyingPeriodContext.php +++ b/src/Bundle/ChillActivityBundle/Service/DocGenerator/ListActivitiesByAccompanyingPeriodContext.php @@ -120,7 +120,6 @@ class ListActivitiesByAccompanyingPeriodContext implements public function contextGenerationDataDenormalize(DocGeneratorTemplate $template, $entity, array $data): array { - $denormalized = $this->accompanyingPeriodContext->contextGenerationDataDenormalize($template, $entity, $data); foreach (['myActivitiesOnly', 'myWorksOnly'] as $k) { diff --git a/src/Bundle/ChillMainBundle/Controller/AddressToReferenceMatcherController.php b/src/Bundle/ChillMainBundle/Controller/AddressToReferenceMatcherController.php index 5cdefceb5..aab63aea3 100644 --- a/src/Bundle/ChillMainBundle/Controller/AddressToReferenceMatcherController.php +++ b/src/Bundle/ChillMainBundle/Controller/AddressToReferenceMatcherController.php @@ -107,5 +107,4 @@ class AddressToReferenceMatcherController true ); } - } diff --git a/src/Bundle/ChillMainBundle/Form/Type/ScopePickerType.php b/src/Bundle/ChillMainBundle/Form/Type/ScopePickerType.php index 6f1a5837f..2529a0655 100644 --- a/src/Bundle/ChillMainBundle/Form/Type/ScopePickerType.php +++ b/src/Bundle/ChillMainBundle/Form/Type/ScopePickerType.php @@ -70,7 +70,8 @@ class ScopePickerType extends AbstractType static function (Scope $s) { return $s->isActive(); } - )); + ) + ); if (0 === count($items)) { throw new RuntimeException('no scopes are reachable. This form should not be shown to user'); diff --git a/src/Bundle/ChillMainBundle/Form/WorkflowStepType.php b/src/Bundle/ChillMainBundle/Form/WorkflowStepType.php index 16dc0a4a5..54d953016 100644 --- a/src/Bundle/ChillMainBundle/Form/WorkflowStepType.php +++ b/src/Bundle/ChillMainBundle/Form/WorkflowStepType.php @@ -247,7 +247,7 @@ class WorkflowStepType extends AbstractType function ($step, ExecutionContextInterface $context, $payload) { $form = $context->getObject(); - foreach($form->get('future_dest_users')->getData() as $u) { + foreach ($form->get('future_dest_users')->getData() as $u) { if (in_array($u, $form->get('future_cc_users')->getData(), true)) { $context ->buildViolation('workflow.The user in cc cannot be a dest user in the same workflow step') diff --git a/src/Bundle/ChillMainBundle/migrations/Version20230306145728.php b/src/Bundle/ChillMainBundle/migrations/Version20230306145728.php index 09a0ece6e..32e138f54 100644 --- a/src/Bundle/ChillMainBundle/migrations/Version20230306145728.php +++ b/src/Bundle/ChillMainBundle/migrations/Version20230306145728.php @@ -69,7 +69,6 @@ final class Version20230306145728 extends AbstractMigration $this->addSql('CREATE INDEX IDX_165051F63174800F ON chill_main_address (createdBy_id)'); $this->addSql('CREATE INDEX IDX_165051F665FF1AEC ON chill_main_address (updatedBy_id)'); $this->addSql('COMMENT ON COLUMN chill_main_address_reference.point IS \'(DC2Type:point)\''); - } public function down(Schema $schema): void diff --git a/src/Bundle/ChillTaskBundle/Controller/SingleTaskController.php b/src/Bundle/ChillTaskBundle/Controller/SingleTaskController.php index 0cdcd7173..2ba9488b6 100644 --- a/src/Bundle/ChillTaskBundle/Controller/SingleTaskController.php +++ b/src/Bundle/ChillTaskBundle/Controller/SingleTaskController.php @@ -685,6 +685,7 @@ final class SingleTaskController extends AbstractController /** * Creates a form to delete a Task entity by id. + * @param mixed $id */ private function createDeleteForm($id): FormInterface {