From 8192531c04f74f263bc2b48fd93b64f83b82653b Mon Sep 17 00:00:00 2001 From: lebaudantoine Date: Tue, 17 Dec 2024 17:02:47 +0100 Subject: [PATCH 1/3] =?UTF-8?q?=E2=9E=95(backend)=20install=20brevo=20clie?= =?UTF-8?q?nt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brevo (ex-sendinblue) is a common solution used for marketing and communications. --- src/backend/pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/backend/pyproject.toml b/src/backend/pyproject.toml index 78a6b89f..0cffe9b0 100644 --- a/src/backend/pyproject.toml +++ b/src/backend/pyproject.toml @@ -27,6 +27,7 @@ requires-python = ">=3.10" dependencies = [ "boto3==1.35.90", "Brotli==1.1.0", + "brevo-python==1.1.2", "celery[redis]==5.4.0", "django-configurations==2.5.1", "django-cors-headers==4.6.0", From ac52c55db7248c5b9556ea23e35112a5ab7a04cf Mon Sep 17 00:00:00 2001 From: lebaudantoine Date: Wed, 18 Dec 2024 10:07:36 +0100 Subject: [PATCH 2/3] =?UTF-8?q?=E2=99=BB=EF=B8=8F(backend)=20add=20Marketi?= =?UTF-8?q?ngService=20protocol=20and=20Brevo=20implementation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduced a MarketingService protocol for typed marketing operations, allowing easy integration of alternative services. Implemented a Brevo wrapper following the protocol to decouple the codebase from the sdk. These implementations are simple and pragmatic. Feel free to refactor them. --- src/backend/core/services/__init__.py | 0 .../core/services/marketing_service.py | 134 +++++++++++++ src/backend/core/tests/services/__init__.py | 0 .../tests/services/test_marketing_service.py | 187 ++++++++++++++++++ src/backend/meet/settings.py | 17 ++ 5 files changed, 338 insertions(+) create mode 100644 src/backend/core/services/__init__.py create mode 100644 src/backend/core/services/marketing_service.py create mode 100644 src/backend/core/tests/services/__init__.py create mode 100644 src/backend/core/tests/services/test_marketing_service.py diff --git a/src/backend/core/services/__init__.py b/src/backend/core/services/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/backend/core/services/marketing_service.py b/src/backend/core/services/marketing_service.py new file mode 100644 index 00000000..a8c8fc8f --- /dev/null +++ b/src/backend/core/services/marketing_service.py @@ -0,0 +1,134 @@ +"""Marketing service in charge of pushing data for marketing automation.""" + +import logging +from dataclasses import dataclass +from functools import lru_cache +from typing import Dict, List, Optional, Protocol + +from django.conf import settings +from django.core.exceptions import ImproperlyConfigured +from django.utils.module_loading import import_string + +import brevo_python + +logger = logging.getLogger(__name__) + + +class ContactCreationError(Exception): + """Raised when the contact creation fails.""" + + +@dataclass +class ContactData: + """Contact data for marketing service integration.""" + + email: str + attributes: Optional[Dict[str, str]] = None + list_ids: Optional[List[int]] = None + update_enabled: bool = True + + +class MarketingServiceProtocol(Protocol): + """Interface for marketing automation service integrations.""" + + def create_contact( + self, contact_data: ContactData, timeout: Optional[int] = None + ) -> dict: + """Create or update a contact. + + Args: + contact_data: Contact information and attributes + timeout: API request timeout in seconds + + Returns: + dict: Service response + + Raises: + ContactCreationError: If contact creation fails + """ + + +class BrevoMarketingService: + """Brevo marketing automation integration. + + Handles: + - Contact management and segmentation + - Marketing campaigns and automation + - Email communications + + Configuration via Django settings: + - BREVO_API_KEY: API authentication + - BREVO_API_CONTACT_LIST_IDS: Default contact lists + - BREVO_API_CONTACT_ATTRIBUTES: Default contact attributes + """ + + def __init__(self): + """Initialize Brevo (ex-sendinblue) marketing service.""" + + if not settings.BREVO_API_KEY: + raise ImproperlyConfigured("Brevo API key is required") + + configuration = brevo_python.Configuration() + configuration.api_key["api-key"] = settings.BREVO_API_KEY + + self._api_client = brevo_python.ApiClient(configuration) + + def create_contact(self, contact_data: ContactData, timeout=None) -> dict: + """Create or update a Brevo contact. + + Args: + contact_data: Contact information and attributes + timeout: API request timeout in seconds + + Returns: + dict: Brevo API response + + Raises: + ContactCreationError: If contact creation fails + ImproperlyConfigured: If required settings are missing + + Note: + Contact attributes must be pre-configured in Brevo. + Changes to attributes can impact existing workflows. + """ + + if not settings.BREVO_API_CONTACT_LIST_IDS: + raise ImproperlyConfigured( + "Default Brevo List IDs must be configured in settings." + ) + + contact_api = brevo_python.ContactsApi(self._api_client) + + attributes = { + **settings.BREVO_API_CONTACT_ATTRIBUTES, + **(contact_data.attributes or {}), + } + + list_ids = (contact_data.list_ids or []) + settings.BREVO_API_CONTACT_LIST_IDS + + contact = brevo_python.CreateContact( + email=contact_data.email, + attributes=attributes, + list_ids=list_ids, + update_enabled=contact_data.update_enabled, + ) + + api_configurations = {} + + if timeout is not None: + api_configurations["_request_timeout"] = timeout + + try: + response = contact_api.create_contact(contact, **api_configurations) + except brevo_python.rest.ApiException as err: + logger.exception("Failed to create contact in Brevo") + raise ContactCreationError("Failed to create contact in Brevo") from err + + return response + + +@lru_cache(maxsize=1) +def get_marketing_service() -> MarketingServiceProtocol: + """Return cached instance of configured marketing service.""" + marketing_service_cls = import_string(settings.MARKETING_SERVICE_CLASS) + return marketing_service_cls() diff --git a/src/backend/core/tests/services/__init__.py b/src/backend/core/tests/services/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/backend/core/tests/services/test_marketing_service.py b/src/backend/core/tests/services/test_marketing_service.py new file mode 100644 index 00000000..910fbae5 --- /dev/null +++ b/src/backend/core/tests/services/test_marketing_service.py @@ -0,0 +1,187 @@ +""" +Test marketing services. +""" + +# pylint: disable=W0621,W0613 + +from unittest import mock + +from django.conf import settings +from django.core.exceptions import ImproperlyConfigured + +import brevo_python +import pytest + +from core.services.marketing_service import ( + BrevoMarketingService, + ContactCreationError, + ContactData, + get_marketing_service, +) + + +def test_init_missing_api_key(settings): + """Test initialization with missing API key.""" + settings.BREVO_API_KEY = None + with pytest.raises(ImproperlyConfigured, match="Brevo API key is required"): + BrevoMarketingService() + + +def test_create_contact_missing_list_ids(settings): + """Test contact creation with missing list IDs.""" + + settings.BREVO_API_KEY = "test-api-key" + settings.BREVO_API_CONTACT_LIST_IDS = None + settings.BREVO_API_CONTACT_ATTRIBUTES = {"source": "test"} + + valid_contact_data = ContactData( + email="test@example.com", + attributes={"first_name": "Test"}, + list_ids=[1, 2], + update_enabled=True, + ) + + brevo_service = BrevoMarketingService() + + with pytest.raises( + ImproperlyConfigured, match="Default Brevo List IDs must be configured" + ): + brevo_service.create_contact(valid_contact_data) + + +@mock.patch("brevo_python.ContactsApi") +def test_create_contact_success(mock_contact_api): + """Test successful contact creation.""" + + mock_api = mock_contact_api.return_value + + settings.BREVO_API_KEY = "test-api-key" + settings.BREVO_API_CONTACT_LIST_IDS = [1, 2, 3, 4] + settings.BREVO_API_CONTACT_ATTRIBUTES = {"source": "test"} + + valid_contact_data = ContactData( + email="test@example.com", + attributes={"first_name": "Test"}, + list_ids=[1, 2], + update_enabled=True, + ) + + brevo_service = BrevoMarketingService() + + mock_api.create_contact.return_value = {"id": "test-id"} + response = brevo_service.create_contact(valid_contact_data) + + assert response == {"id": "test-id"} + + mock_api.create_contact.assert_called_once() + contact_arg = mock_api.create_contact.call_args[0][0] + assert contact_arg.email == "test@example.com" + assert contact_arg.attributes == { + **settings.BREVO_API_CONTACT_ATTRIBUTES, + **valid_contact_data.attributes, + } + assert set(contact_arg.list_ids) == {1, 2, 3, 4} + assert contact_arg.update_enabled is True + + +@mock.patch("brevo_python.ContactsApi") +def test_create_contact_with_timeout(mock_contact_api): + """Test contact creation with timeout.""" + + mock_api = mock_contact_api.return_value + + settings.BREVO_API_KEY = "test-api-key" + settings.BREVO_API_CONTACT_LIST_IDS = [1, 2, 3, 4] + settings.BREVO_API_CONTACT_ATTRIBUTES = {"source": "test"} + + valid_contact_data = ContactData( + email="test@example.com", + attributes={"first_name": "Test"}, + list_ids=[1, 2], + update_enabled=True, + ) + + brevo_service = BrevoMarketingService() + brevo_service.create_contact(valid_contact_data, timeout=30) + + mock_api.create_contact.assert_called_once() + assert mock_api.create_contact.call_args[1]["_request_timeout"] == 30 + + +@mock.patch("brevo_python.ContactsApi") +def test_create_contact_api_error(mock_contact_api): + """Test contact creation API error handling.""" + + mock_api = mock_contact_api.return_value + + settings.BREVO_API_KEY = "test-api-key" + settings.BREVO_API_CONTACT_LIST_IDS = [1, 2, 3, 4] + settings.BREVO_API_CONTACT_ATTRIBUTES = {"source": "test"} + + valid_contact_data = ContactData( + email="test@example.com", + attributes={"first_name": "Test"}, + list_ids=[1, 2], + update_enabled=True, + ) + + brevo_service = BrevoMarketingService() + + mock_api.create_contact.side_effect = brevo_python.rest.ApiException() + + with pytest.raises(ContactCreationError, match="Failed to create contact in Brevo"): + brevo_service.create_contact(valid_contact_data) + + +@pytest.fixture +def clear_marketing_cache(): + """Clear marketing service cache between tests.""" + get_marketing_service.cache_clear() + yield + get_marketing_service.cache_clear() + + +def test_get_marketing_service_caching(clear_marketing_cache): + """Test marketing service caching behavior.""" + settings.BREVO_API_KEY = "test-api-key" + settings.MARKETING_SERVICE_CLASS = ( + "core.services.marketing_service.BrevoMarketingService" + ) + + service1 = get_marketing_service() + service2 = get_marketing_service() + + assert service1 is service2 + assert isinstance(service1, BrevoMarketingService) + + +def test_get_marketing_service_invalid_class(clear_marketing_cache): + """Test handling of invalid service class.""" + settings.MARKETING_SERVICE_CLASS = "invalid.service.path" + + with pytest.raises(ImportError): + get_marketing_service() + + +@mock.patch("core.services.marketing_service.import_string") +def test_service_instantiation_called_once(mock_import_string, clear_marketing_cache): + """Test service class is instantiated only once.""" + + settings.BREVO_API_KEY = "test-api-key" + settings.MARKETING_SERVICE_CLASS = ( + "core.services.marketing_service.BrevoMarketingService" + ) + get_marketing_service.cache_clear() + + mock_service_cls = mock.Mock() + mock_service_instance = mock.Mock() + mock_service_cls.return_value = mock_service_instance + mock_import_string.return_value = mock_service_cls + + service1 = get_marketing_service() + service2 = get_marketing_service() + + mock_import_string.assert_called_once_with(settings.MARKETING_SERVICE_CLASS) + mock_service_cls.assert_called_once() + assert service1 is service2 + assert service1 is mock_service_instance diff --git a/src/backend/meet/settings.py b/src/backend/meet/settings.py index dfaadea5..6dd52171 100755 --- a/src/backend/meet/settings.py +++ b/src/backend/meet/settings.py @@ -457,6 +457,23 @@ class Base(Configuration): None, environ_name="SUMMARY_SERVICE_API_TOKEN", environ_prefix=None ) + # Marketing and communication settings + MARKETING_SERVICE_CLASS = values.Value( + "core.services.marketing_service.BrevoMarketingService", + environ_name="MARKETING_SERVICE_CLASS", + environ_prefix=None, + ) + BREVO_API_KEY = values.Value( + None, environ_name="BREVO_API_KEY", environ_prefix=None + ) + BREVO_API_CONTACT_LIST_IDS = values.ListValue( + [], + environ_name="BREVO_API_CONTACT_LIST_IDS", + environ_prefix=None, + converter=lambda x: int(x), # pylint: disable=unnecessary-lambda + ) + BREVO_API_CONTACT_ATTRIBUTES = values.DictValue({"VISIO_USER": True}) + # pylint: disable=invalid-name @property def ENVIRONMENT(self): From 413adc9d0239dfa1337e429dd5180adb7fae1e9c Mon Sep 17 00:00:00 2001 From: lebaudantoine Date: Fri, 20 Dec 2024 15:40:34 +0100 Subject: [PATCH 3/3] =?UTF-8?q?=E2=9C=A8(backend)=20post=20email=20to=20ma?= =?UTF-8?q?rketing=20tools=20while=20signing=20up=20new=20users?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Submitting new users to the marketing service is currently handled during signup and is performed only once. This is a pragmatic first implementation, yet imperfect. In the future, this should be improved by delegating the call to a Celery worker or an async task. --- src/backend/core/authentication/backends.py | 31 +++- .../tests/authentication/test_backends.py | 141 +++++++++++++++++- src/backend/meet/settings.py | 9 ++ 3 files changed, 179 insertions(+), 2 deletions(-) diff --git a/src/backend/core/authentication/backends.py b/src/backend/core/authentication/backends.py index 202a5293..46fa366f 100644 --- a/src/backend/core/authentication/backends.py +++ b/src/backend/core/authentication/backends.py @@ -1,7 +1,7 @@ """Authentication Backends for the Meet core app.""" from django.conf import settings -from django.core.exceptions import SuspiciousOperation +from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation from django.utils.translation import gettext_lazy as _ import requests @@ -10,6 +10,11 @@ ) from core.models import User +from core.services.marketing_service import ( + ContactCreationError, + ContactData, + get_marketing_service, +) class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend): @@ -86,6 +91,10 @@ def get_or_create_user(self, access_token, id_token, payload): password="!", # noqa: S106 **claims, ) + + if settings.SIGNUP_NEW_USER_TO_MARKETING_EMAIL: + self.signup_to_marketing_email(email) + elif not user: return None @@ -96,6 +105,26 @@ def get_or_create_user(self, access_token, id_token, payload): return user + @staticmethod + def signup_to_marketing_email(email): + """Pragmatic approach to newsletter signup during authentication flow. + + Details: + 1. Uses a very short timeout (1s) to prevent blocking the auth process + 2. Silently fails if the marketing service is down/slow to prioritize user experience + 3. Trade-off: May miss some signups but ensures auth flow remains fast + + Note: For a more robust solution, consider using Async task processing (Celery/Django-Q) + """ + try: + marketing_service = get_marketing_service() + contact_data = ContactData( + email=email, attributes={"VISIO_SOURCE": ["SIGNIN"]} + ) + marketing_service.create_contact(contact_data, timeout=1) + except (ContactCreationError, ImproperlyConfigured, ImportError): + pass + def get_existing_user(self, sub, email): """Fetch existing user by sub or email.""" try: diff --git a/src/backend/core/tests/authentication/test_backends.py b/src/backend/core/tests/authentication/test_backends.py index 280caa37..750b46c0 100644 --- a/src/backend/core/tests/authentication/test_backends.py +++ b/src/backend/core/tests/authentication/test_backends.py @@ -1,12 +1,15 @@ """Unit tests for the Authentication Backends.""" -from django.core.exceptions import SuspiciousOperation +from unittest import mock + +from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation import pytest from core import models from core.authentication.backends import OIDCAuthenticationBackend from core.factories import UserFactory +from core.services import marketing_service pytestmark = pytest.mark.django_db @@ -412,3 +415,139 @@ def test_update_user_when_no_update_needed(django_assert_num_queries, claims): user.refresh_from_db() assert user.email == "john.doe@example.com" + + +@mock.patch.object(OIDCAuthenticationBackend, "signup_to_marketing_email") +def test_marketing_signup_new_user_enabled(mock_signup, monkeypatch, settings): + """Test marketing signup for new user with settings enabled.""" + settings.SIGNUP_NEW_USER_TO_MARKETING_EMAIL = True + + klass = OIDCAuthenticationBackend() + email = "test@example.com" + + def get_userinfo_mocked(*args): + return {"sub": "123", "email": email} + + monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked) + + user = klass.get_or_create_user("test-token", None, None) + + assert user.email == email + mock_signup.assert_called_once_with(email) + + +@mock.patch.object(OIDCAuthenticationBackend, "signup_to_marketing_email") +def test_marketing_signup_new_user_disabled(mock_signup, monkeypatch, settings): + """Test no marketing signup for new user with settings disabled.""" + settings.SIGNUP_NEW_USER_TO_MARKETING_EMAIL = False + + klass = OIDCAuthenticationBackend() + email = "test@example.com" + + def get_userinfo_mocked(*args): + return {"sub": "123", "email": email} + + monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked) + + user = klass.get_or_create_user("test-token", None, None) + + assert user.email == email + mock_signup.assert_not_called() + + +@mock.patch.object(OIDCAuthenticationBackend, "signup_to_marketing_email") +def test_marketing_signup_new_user_default_disabled(mock_signup, monkeypatch): + """Test no marketing signup for new user with settings by default disabled.""" + + klass = OIDCAuthenticationBackend() + email = "test@example.com" + + def get_userinfo_mocked(*args): + return {"sub": "123", "email": email} + + monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked) + + user = klass.get_or_create_user("test-token", None, None) + + assert user.email == email + mock_signup.assert_not_called() + + +@pytest.mark.parametrize( + "is_signup_enabled", + [True, False], +) +@mock.patch.object(OIDCAuthenticationBackend, "signup_to_marketing_email") +def test_marketing_signup_existing_user( + mock_signup, monkeypatch, settings, is_signup_enabled +): + """Test no marketing signup for existing user regardless of settings.""" + + settings.SIGNUP_NEW_USER_TO_MARKETING_EMAIL = is_signup_enabled + + klass = OIDCAuthenticationBackend() + db_user = UserFactory(email="test@example.com") + + def get_userinfo_mocked(*args): + return {"sub": db_user.sub, "email": db_user.email} + + monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked) + + user = klass.get_or_create_user("test-token", None, None) + assert user == db_user + mock_signup.assert_not_called() + + +@mock.patch("core.authentication.backends.get_marketing_service") +def test_signup_to_marketing_email_success(mock_marketing): + """Test successful marketing signup.""" + + email = "test@example.com" + + # Call the method + OIDCAuthenticationBackend.signup_to_marketing_email(email) + + # Verify service interaction + mock_service = mock_marketing.return_value + mock_service.create_contact.assert_called_once() + + +@pytest.mark.parametrize( + "error", + [ + ImportError, + ImproperlyConfigured, + ], +) +@mock.patch("core.authentication.backends.get_marketing_service") +def test_marketing_signup_handles_service_initialization_errors( + mock_marketing, error, settings +): + """Tests errors that occur when trying to get/initialize the marketing service.""" + settings.SIGNUP_NEW_USER_TO_MARKETING_EMAIL = True + + mock_marketing.side_effect = error + + # Should not raise any exception + OIDCAuthenticationBackend.signup_to_marketing_email("test@example.com") + + +@pytest.mark.parametrize( + "error", + [ + marketing_service.ContactCreationError, + ImproperlyConfigured, + ImportError, + ], +) +@mock.patch("core.authentication.backends.get_marketing_service") +def test_marketing_signup_handles_contact_creation_errors( + mock_marketing, error, settings +): + """Tests errors that occur during the contact creation process.""" + + settings.SIGNUP_NEW_USER_TO_MARKETING_EMAIL = True + mock_marketing.return_value.create_contact.side_effect = error + + # Should not raise any exception + OIDCAuthenticationBackend.signup_to_marketing_email("test@example.com") diff --git a/src/backend/meet/settings.py b/src/backend/meet/settings.py index 6dd52171..9b30d2de 100755 --- a/src/backend/meet/settings.py +++ b/src/backend/meet/settings.py @@ -458,6 +458,15 @@ class Base(Configuration): ) # Marketing and communication settings + SIGNUP_NEW_USER_TO_MARKETING_EMAIL = values.BooleanValue( + False, + environ_name="SIGNUP_NEW_USERS_TO_NEWSLETTER", + environ_prefix=None, + help_text=( + "When enabled, new users are automatically added to mailing list " + "for product updates, marketing communications, and customized emails. " + ), + ) MARKETING_SERVICE_CLASS = values.Value( "core.services.marketing_service.BrevoMarketingService", environ_name="MARKETING_SERVICE_CLASS",