-
Notifications
You must be signed in to change notification settings - Fork 2
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
base: master
Are you sure you want to change the base?
Conversation
… update_contact_email_blacklisted function
@@ -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): |
There was a problem hiding this comment.
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.
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) |
There was a problem hiding this comment.
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.
response = self.client.post( | ||
self.url, | ||
{ | ||
"email_group_1": True, |
There was a problem hiding this comment.
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
self.url, | ||
{ | ||
"email_group_1": True, | ||
"email_group_2": True, |
There was a problem hiding this comment.
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" ?
Quoi ?
Le choix concernant les mails "Communication Marketing" est poussé côté Brevo.
Pourquoi ?
Pour que les emails des campagnes ne partent plus vers les utilisateurs qui ne le souhaitent plus.
Comment ?
Avec l'API Brevo et le champs
email_blacklisted
Captures d'écran
Côté Brevo
Autre
À noter que la désinscription aux envois de mail faite côté Brevo (en bas des mails par exemple), n'est pas répercuté de notre côté. À voir si il faut faire un script de synchro ou juste aller chercher l'info avant d'afficher les formulaires.