From 47c0af3623d47703571953a8c06eeb45095f1836 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Tue, 21 Jan 2025 22:39:37 +0100 Subject: [PATCH 1/3] Handle missing message IDs in SentMessageEventSubscriber Added a condition to log an info message when the sent SMS lacks a message ID. Ensures clearer distinction between successful and incomplete SMS message logging. --- .../Service/Notifier/SentMessageEventSubscriber.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Bundle/ChillMainBundle/Service/Notifier/SentMessageEventSubscriber.php b/src/Bundle/ChillMainBundle/Service/Notifier/SentMessageEventSubscriber.php index ba94fba49..2d003d775 100644 --- a/src/Bundle/ChillMainBundle/Service/Notifier/SentMessageEventSubscriber.php +++ b/src/Bundle/ChillMainBundle/Service/Notifier/SentMessageEventSubscriber.php @@ -32,6 +32,10 @@ final readonly class SentMessageEventSubscriber implements EventSubscriberInterf { $message = $event->getMessage(); - $this->logger->warning('[sms] a sms was sent', ['validReceiversI' => $message->getOriginalMessage()->getRecipientId(), 'idsI' => $message->getMessageId()]); + if (null === $message->getMessageId()) { + $this->logger->info('[sms] a sms message did not had any id after sending.', ['validReceiversI' => $message->getOriginalMessage()->getRecipientId()]); + } else { + $this->logger->warning('[sms] a sms was sent', ['validReceiversI' => $message->getOriginalMessage()->getRecipientId(), 'idsI' => $message->getMessageId()]); + } } } From ceb0bd982e8d66612bc657d917242caee0073153 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Tue, 21 Jan 2025 22:41:39 +0100 Subject: [PATCH 2/3] Prevent default texter configuration if already defined Check existing framework configurations for `texter_transports` before applying default settings. This avoids overwriting or conflicting with pre-defined notifier configurations. --- .changes/unreleased/Fixed-20250121-225838.yaml | 6 ++++++ .../DependencyInjection/ChillMainExtension.php | 11 +++++++++++ 2 files changed, 17 insertions(+) create mode 100644 .changes/unreleased/Fixed-20250121-225838.yaml diff --git a/.changes/unreleased/Fixed-20250121-225838.yaml b/.changes/unreleased/Fixed-20250121-225838.yaml new file mode 100644 index 000000000..f22ed2697 --- /dev/null +++ b/.changes/unreleased/Fixed-20250121-225838.yaml @@ -0,0 +1,6 @@ +kind: Fixed +body: Fix legacy configuration processor for notifier component +time: 2025-01-21T22:58:38.805800688+01:00 +custom: + Issue: "" + SchemaChange: No schema change diff --git a/src/Bundle/ChillMainBundle/DependencyInjection/ChillMainExtension.php b/src/Bundle/ChillMainBundle/DependencyInjection/ChillMainExtension.php index 2e9b7c029..9fa3b0f42 100644 --- a/src/Bundle/ChillMainBundle/DependencyInjection/ChillMainExtension.php +++ b/src/Bundle/ChillMainBundle/DependencyInjection/ChillMainExtension.php @@ -366,6 +366,17 @@ class ChillMainExtension extends Extension implements */ private function prependNotifierTexterWithLegacyData(ContainerBuilder $container): void { + foreach (array_reverse($container->getExtensionConfig('framework')) as $c) { + // we look into each configuration for framework. If there is a configuration for + // texter_transports in one of them, we don't configure anything else + if (null !== $notifConfig = $c['notifier'] ?? null) { + if (null !== ($notifConfig['texter_transports'] ?? null)) { + return; + } + } + } + + // there is no texter config, we try to configure one $configs = $container->getExtensionConfig('chill_main'); $notifierSet = false; foreach (array_reverse($configs) as $config) { From be5655e537f7e7a72cd00c0e18c95c8d3f742b48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Tue, 21 Jan 2025 22:50:06 +0100 Subject: [PATCH 3/3] Refine Notifier component configuration guidance Added detailed instructions to configure the Symfony Notifier component correctly, emphasizing proper setup for SMS providers like OVHcloud. Clarified removal of legacy configurations and included examples for both utilizing and replacing the added configuration. --- .changes/v3.7.0.md | 61 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/.changes/v3.7.0.md b/.changes/v3.7.0.md index 4fbe5d213..34e4ca4ed 100644 --- a/.changes/v3.7.0.md +++ b/.changes/v3.7.0.md @@ -1,5 +1,62 @@ ## v3.7.0 - 2025-01-21 ### Feature -* Use the Notifier component from Symfony to sens short messages (SMS). This allow to use more provider. +* Use the Notifier component from Symfony to sens short messages (SMS). This allow to use more provider. ### Fixed -* ([#348](https://gitlab.com/Chill-Projet/chill-bundles/-/issues/348)) [export] Fix aggregation of referrer's scope and job: fix the date range comparison +* ([#348](https://gitlab.com/Chill-Projet/chill-bundles/-/issues/348)) [export] Fix aggregation of referrer's scope and job: fix the date range comparison + +### Warning on configuration of Notifier component + +If installed in an symfony app where the recipes are activated, this configuration should be added automatically: + +```yaml +framework: + notifier: + chatter_transports: + texter_transports: + ovhcloud: '%env(OVHCLOUD_DSN)%' + channel_policy: + # use chat/slack, chat/telegram, sms/twilio or sms/nexmo + urgent: ['email'] + high: ['email'] + medium: ['email'] + low: ['email'] + admin_recipients: + - { email: admin@example.com } +``` + +Actually, you should either: + +- remove the configuration of ovhcloud added by the recipe +- or remove the previous configuration of chill, to avoid keeping legacy configuration + +#### Remove the added configuration and keep the legacy configuration + +To remove the configuration: + +```diff +framework: + notifier: + chatter_transports: + texter_transports: +- ovhcloud: '%env(OVHCLOUD_DSN)%' +``` + +In that case, the previous configuration, which was stored under the `chill_main.short_messages.dsn` will be reconfigured into the Notifier component's configuration. + +#### Properly configure SMS + +You can also properly configure it, as [described in the OVH cloud provider repository](https://github.com/symfony/ovh-cloud-notifier/tree/5.4?tab=readme-ov-file#dsn-example) (where the scheme is `ovhcloud`): + +**NOTE**: You have access to all notifier available with the [Notifier component](https://symfony.com/doc/current/notifier.html#notifier-sms-channel). You are not restricted to use OVH as a provider. + +```diff +framework: + notifier: + chatter_transports: + texter_transports: ++ ovhcloud: '%env(OVHCLOUD_DSN)%' # this value should be located in a variable, and have `ovhcloud://` as a scheme + +chill_main: +- short_messages: +- dsn: '%env(string:SHORT_MESSAGE_DSN)%' +```