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

Filtrer les emails non valides avant l'envoie #1645

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

kolok
Copy link
Collaborator

@kolok kolok commented Nov 12, 2024

Description succincte du problème résolu

Erreur Sentry : https://sentry.incubateur.net/organizations/betagouv/issues/130638/?environment=production&project=29&query=is%3Aunresolved&referrer=issue-stream&statsPeriod=14d&stream_index=5

Certain utilisateur n'ont pas d'email correct, on ignore ces emails et on log un wanning

Type de changement :

  • Bug fix
  • Nouvelle fonctionnalité
  • Mise à jour de données / DAG
  • Les changements nécessitent une mise à jour de documentation
  • Refactoring de code (explication à retrouver dans la description)

Auto-review

Les trucs à faire avant de demander une review :

  • J'ai bien relu mon code
  • La CI passe bien
  • En cas d'ajout de variable d'environnement, j'ai bien mis à jour le .env.template
  • J'ai ajouté des tests qui couvrent le nouveau code

Comment tester

En local / staging :

@kolok kolok requested a review from a team as a code owner November 12, 2024 10:19
@kolok kolok requested review from etchegom and syldb and removed request for a team November 12, 2024 10:19
Copy link

sonarcloud bot commented Nov 12, 2024

Copy link

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
8319 6917 83% 80% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
core/services.py 95% 🟢
TOTAL 95% 🟢

updated for commit: 32220dd by action🐍

invalid_emails = set(self.to_emails) - set(valid_to_emails)

if not valid_to_emails:
raise Exception("No valid recipient for email")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise Exception("No valid recipient for email")
raise ValueError("No valid recipient for email")

@@ -25,6 +26,25 @@ def test_basic_send_transactional_email(self):
"email_param_value",
)

def test_send_transactional_email_no_emails_valid(self):
with pytest.raises(Exception, match="recipient for email"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
with pytest.raises(Exception, match="recipient for email"):
with pytest.raises(ValueError, match="recipient for email"):

Copy link
Contributor

@etchegom etchegom left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants