Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Email Notifications): Mise à jour côté Brevo du choix pour les communications Marketing #1639

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lemarche/conversations/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,8 @@ def set_validated(self):


class EmailGroup(models.Model):
COMMUNICATION_MARKETING = "Communication marketing"

display_name = models.CharField(verbose_name="Nom", max_length=255, blank=True)
description = models.TextField(verbose_name="Description", blank=True)
relevant_user_kind = models.CharField(
Expand Down
12 changes: 12 additions & 0 deletions lemarche/utils/apis/api_brevo.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,18 @@ def update_contact(user_identifier: str, attributes_to_update: dict):
logger.error(f"Exception when calling Brevo->ContactsApi->update_contact: {e}")


def update_contact_email_blacklisted(user_identifier: str, email_blacklisted: bool):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seuls les message du logger et l'argument vers .UpdateContact() changent par rapport à la fonction du dessus, update_contact. Il faudrait peut être factoriser cela.

api_client = get_api_client()
api_instance = sib_api_v3_sdk.ContactsApi(api_client)

update_contact = sib_api_v3_sdk.UpdateContact(email_blacklisted=email_blacklisted)
try:
api_response = api_instance.update_contact(identifier=user_identifier, update_contact=update_contact)
logger.info(f"Success Brevo->ContactsApi->update_contact to update email_blacklisted: {api_response}")
except ApiException as e:
logger.error(f"Exception when calling Brevo->ContactsApi->update_contact to update email_blacklisted: {e}")


def remove_contact_from_list(user, list_id: int):
api_client = get_api_client()
api_instance = sib_api_v3_sdk.ContactsApi(api_client)
Expand Down
4 changes: 4 additions & 0 deletions lemarche/utils/emails.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ def add_to_contact_list(user, type: str, tender=None, source: str = user_constan
api_mailjet.add_to_contact_list_async(user.email, properties, contact_list_id)


def update_contact_email_blacklisted(email, email_blacklisted: bool):
api_brevo.update_contact_email_blacklisted(email, email_blacklisted)


@task()
def send_mail_async(
email_subject,
Expand Down
18 changes: 13 additions & 5 deletions lemarche/www/dashboard/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from lemarche.conversations.models import DisabledEmail, EmailGroup
from lemarche.sectors.models import Sector
from lemarche.users.models import User
from lemarche.utils.emails import update_contact_email_blacklisted
from lemarche.utils.fields import GroupedModelMultipleChoiceField
from lemarche.utils.widgets import CustomSelectMultiple

Expand Down Expand Up @@ -55,14 +56,21 @@ def __init__(self, *args, **kwargs):
def save(self):
disabled_emails = []

disabled_emails_marketing = False

# add unchecked fields to disabled_emails
for field_name, value in self.cleaned_data.items():
if field_name.startswith("email_group_"):
if not value:
group = EmailGroup.objects.get(pk=int(field_name.replace("email_group_", "")))
disabled_email, _ = DisabledEmail.objects.get_or_create(user=self.user, group=group)
disabled_emails.append(disabled_email)
if field_name.startswith("email_group_") and not value:
group = EmailGroup.objects.get(pk=int(field_name.replace("email_group_", "")))
disabled_email, _ = DisabledEmail.objects.get_or_create(user=self.user, group=group)
disabled_emails.append(disabled_email)

if group.display_name == EmailGroup.COMMUNICATION_MARKETING:
disabled_emails_marketing = True

self.user.disabled_emails.set(disabled_emails)

# remove old disabled_emails
DisabledEmail.objects.exclude(pk__in=[de.pk for de in disabled_emails]).delete()

update_contact_email_blacklisted(self.user.email, disabled_emails_marketing)
41 changes: 40 additions & 1 deletion lemarche/www/dashboard/tests.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from unittest.mock import patch

from django.test import TestCase
from django.urls import reverse

Expand Down Expand Up @@ -88,7 +90,11 @@ def test_get_form_displays_correctly(self):
self.assertContains(response, group.display_name)
self.assertContains(response, " checked>", count=2)

def test_form_submission_updates_preferences(self):
@patch("lemarche.utils.apis.api_brevo.sib_api_v3_sdk.ContactsApi")
def test_form_submission_updates_preferences_with_marketing_disabled(self, mock_contacts_api):
# Setup the mock
mock_api_instance = mock_contacts_api.return_value

self.assertEqual(self.user.disabled_emails.count(), 0)
self.client.force_login(self.user)
response = self.client.post(
Expand All @@ -99,7 +105,40 @@ def test_form_submission_updates_preferences(self):
},
follow=True,
)

# Verify the API was called correctly
mock_api_instance.update_contact.assert_called_once()
call_args = mock_api_instance.update_contact.call_args
self.assertEqual(call_args[1]["identifier"], self.user.email)
self.assertEqual(call_args[1]["update_contact"].email_blacklisted, True)

self.assertContains(response, "Vos préférences de notifications ont été mises à jour.")
self.user.refresh_from_db()
self.assertEqual(self.user.disabled_emails.count(), 1)
self.assertEqual(self.user.disabled_emails.first().group.pk, 2)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il faut éviter de tester de cette manière, en faisant .first() on compte sur l'ordre, qui n'est pas toujours garanti ou qu'on pourrait changer sans casser de fonctionnalité. Pareil pour la pk, c'est la base qui le gère, par exemple avec --keepdb les séquences ne sont pas reset.


@patch("lemarche.utils.apis.api_brevo.sib_api_v3_sdk.ContactsApi")
def test_form_submission_updates_preferences_with_marketing_enabled(self, mock_contacts_api):
# Setup the mock
mock_api_instance = mock_contacts_api.return_value

self.assertEqual(self.user.disabled_emails.count(), 0)
self.client.force_login(self.user)
response = self.client.post(
self.url,
{
"email_group_1": True,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'ai mis un peu de temps à comprendre à quoi correspondaient email_group_1 et email_group_2. Avoir des get sur les emailsgroup concernés serait plus clair, genre:

email_group_1 = EmailGroup.objects.get(...)
#...
f"email_group_{email_group_1pk}": True

Et pour les tests ca permettrait d'individualiser lequel est désactivé ou non

"email_group_2": True,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

En fait, "email_group_2" => "Communication marketing" ?

},
follow=True,
)

# Verify the API was called correctly
mock_api_instance.update_contact.assert_called_once()
call_args = mock_api_instance.update_contact.call_args
self.assertEqual(call_args[1]["identifier"], self.user.email)
self.assertEqual(call_args[1]["update_contact"].email_blacklisted, False)

self.assertContains(response, "Vos préférences de notifications ont été mises à jour.")
self.user.refresh_from_db()
self.assertEqual(self.user.disabled_emails.count(), 0)
Loading