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

Fixbug adresse destination invalide (envoi de SMS) #4648

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Shamzic
Copy link
Contributor

@Shamzic Shamzic commented Oct 8, 2024

Certains numéros de SMS sont validés par le front mais ne sont pas valides pour autant en backend. Cela générait un statut waiting à true avec un chargement infini sur le front sans aucune gestion d'erreur.

Exemple de numéro invalide avec chargement infini en cliquant ici +594 696 00 01 02

Cette correction remonte l'erreur "adresse de destination invalide" soulevée par le backend et l'interprète sur le front avec un message spécifique pour éviter le loading infini.

Bug sentry associé

@github-actions github-actions bot added this to the BC actuel milestone Oct 8, 2024
@jenovateurs
Copy link
Contributor

C'est un doublon avec cette PR @Shamzic : #4643 ?

@Shamzic
Copy link
Contributor Author

Shamzic commented Oct 9, 2024

C'est un doublon avec cette PR @Shamzic : #4643 ?

Ça ne corrige pas exactement la même chose, non. Mais y a la partie sur le fichier src/components/modals/errors-email-and-sms-modal.vue qui est similaire. Du coup il faudrait peut-être fusionner les deux solutions. J'ai complété en description le bug identifié qui concerne cette PR.

Copy link
Contributor

@jenovateurs jenovateurs left a comment

Choose a reason for hiding this comment

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

Plus des questions et remarques.

backend/controllers/followups.ts Outdated Show resolved Hide resolved
if (!error?.message?.includes("Invalid destination address")) {
Sentry.captureException(error)
}
followup.smsError = JSON.stringify(error, null, 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Tu as pu tester ? Cela ne va pas faire de doublon json entre ici et la génération de la réponse.

Copy link
Contributor Author

@Shamzic Shamzic Oct 17, 2024

Choose a reason for hiding this comment

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

Je ne sais pas si j'ai bien compris ta remarque, mais normalement oui, la condition sur le Sentry.captureException(error) est effective et j'ai aussi trouvé un problème à l'instant que j'ai corrigé : la sauvegarde du message d'erreur dans le followup est maintenant effective => 2b91c53

src/components/modals/errors-email-and-sms-modal.vue Outdated Show resolved Hide resolved
console.error(error)
Sentry.captureException(error)
} catch (error: any) {
if (!error?.response?.data?.includes("Invalid destination address")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

À voir, s'il ne faudrait pas avoir une constante pour gérer les « codes d'erreur », comme ça on pourrait voir le lien entre l'erreur envoyée en back et reçue en front.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

À mes yeux, le lien est plutôt explicite ici en regardant le contenu de error.response.data, je suis preneur d'un exemple si tu as une meilleure implémentation

@Shamzic Shamzic force-pushed the fix-sms-adresse-destination-invalide branch from 7ee06de to cf65a55 Compare October 17, 2024 16:41
@Shamzic
Copy link
Contributor Author

Shamzic commented Oct 17, 2024

Rebase avec l'intégration du refacto de la #4652

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