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

Envoi du lien du sondage par SMS #4058

Closed
wants to merge 30 commits into from

Conversation

Shamzic
Copy link
Contributor

@Shamzic Shamzic commented Nov 8, 2023

Objectif

Envoyer des SMS contenant le lien du sondage 6 jours après la réalisation d'une simulation (comportement similaire à l'envoi par e-mail).

L'accès au sondage via le lien du SMS se fera par une route de redirection raccourcie (api/r/).

Le sondage est envoyé par SMS selon des conditions spécifiées :

À 7 jours après la création du suivi, si seul le numéro de téléphone est renseigné.
À 10 jours après la création du suivi si le numéro de téléphone et l'e-mail ont été renseignés, que le sondage a été envoyé par e-mail (au bout du 7e jour) mais qu'il n'a pas obtenu de réponses.

Tâche sur l'ops

Ajouter un cron hebdomadaire d'envoi de sondage par SMS similaire à celui par e-mail.
Pull Request ouverte : betagouv/aides-jeunes-ops#176

Refactoring dans une première PR (pour éviter de surcharger celle-ci)

Ici : #4066

Tester la PR

Envoyer des liens de sondage pour tous les suivis (maximum 10 envois) respectant les critères d'envoi ; la commande npm run tools:send-initial-survey-sms en ayant au préalable indiqué les identifiants Netsize en variable d'environnement.

Exécuter les tests unitaires du fichier messaging-sms.spec.ts ciblant la méthode de calcul des critères d'envoi du sondage par SMS. => peut encore être approfondi.

@Shamzic Shamzic self-assigned this Nov 8, 2023
@Shamzic Shamzic force-pushed the feature-envoi-survey-sms branch 6 times, most recently from 2062e4e to e43e3a3 Compare November 10, 2023 14:02
@Shamzic Shamzic marked this pull request as ready for review November 10, 2023 19:13
@Shamzic Shamzic requested a review from a team November 10, 2023 19:13
@Shamzic Shamzic changed the title Ajoute l'envoi de SMS manuel (WIP) Rend possible l'envoi du lien d'accès au sondage par SMS Nov 10, 2023
@guillett
Copy link
Contributor

Est ce plutôt facile pour toi de couper cette PR en deux avec le refactor d'une part et la nouvelle fonctionnalité d'autre part ?

@Shamzic
Copy link
Contributor Author

Shamzic commented Nov 13, 2023

Est ce plutôt facile pour toi de couper cette PR en deux avec le refactor d'une part et la nouvelle fonctionnalité d'autre part ?

Oui j'ai bien en tête le refactor souhaité, c'est ce que j'ai expliqué dans la description mais du coup tu trouves qu'il y a déjà trop de refacto ici ?

@guillett
Copy link
Contributor

J'ai l'impression qu'il y a 50% (voire plus) de refacto et 50% pour la nouvelle fonctionnalité. Sur une PR qui fait +400 - 100 je me dis que ça serait bien de découper en deux.

@Shamzic Shamzic force-pushed the feature-envoi-survey-sms branch 10 times, most recently from ba953c9 to d8e452f Compare November 14, 2023 14:27
@Shamzic
Copy link
Contributor Author

Shamzic commented Nov 14, 2023

J'ai l'impression qu'il y a 50% (voire plus) de refacto et 50% pour la nouvelle fonctionnalité. Sur une PR qui fait +400 - 100 je me dis que ça serait bien de découper en deux.

J'ai déplacé 100 lignes de refacto ici : https://github.com/betagouv/aides-jeunes/pull/4066/files je pense qu'on est au max. ^^
C'est bon pour toi ?

backend/lib/sms-sending-tool.ts Outdated Show resolved Hide resolved
backend/lib/sms-service.ts Outdated Show resolved Hide resolved
type: {
$in: [
SurveyCategory.BenefitAction,
SurveyCategory.TrackClickOnBenefitActionSms,
Copy link
Contributor

Choose a reason for hiding this comment

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

C'est pas vraiment dans les specs la question de "à qui on envoi" mais

  • On veut envoyer email ET sms à ceux qui ont mis les deux ? (DHL le fait pour les livraisons)
  • De manière général si quelqu'un a cliqué sur l'email on veut quand même envoyer le sms ? (on pourrait rajouter SurveyCategory.TrackClickOnSimulationUsefulnessEmail et SurveyCategory.TrackClickOnBenefitActionEmail

Copy link
Contributor Author

@Shamzic Shamzic Nov 15, 2023

Choose a reason for hiding this comment

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

Oui, c'est pas dans les specs et cela reste à définir. J'aurais tendance à découpler les deux envois et faire les deux en même temps si les deux infos sont renseignées.

  • On veut envoyer email ET sms à ceux qui ont mis les deux ? (DHL le fait pour les livraisons)

Oui, j'avais intuitivement cette idée en tête.

  • De manière général si quelqu'un a cliqué sur l'email on veut quand même envoyer le sms ? (on pourrait rajouter SurveyCategory.TrackClickOnSimulationUsefulnessEmail et SurveyCategory.TrackClickOnBenefitActionEmail

Oui sauf si le sondage est déjà répondu (car cela n'aurait pas trop de sens de le renvoyer via un autre moyen de communication)

@guillett Quel est ton avis sur ces questions ?

backend/lib/sms-service.ts Outdated Show resolved Hide resolved
lib/enums/sms.ts Outdated Show resolved Hide resolved
backend/controllers/followups.ts Show resolved Hide resolved
@Shamzic Shamzic force-pushed the feature-envoi-survey-sms branch from 7d25eac to 3fbb1d7 Compare November 16, 2023 14:41
Comment on lines +66 to +76
followup.smsSentAt = Date.now()
followup.smsMessageId = data.messageIds[0]
if (!followup.surveyOptin) {
followup.phone = undefined
}
followup.smsError = undefined
break
case SmsCategory.InitialSurvey:
followup.smsSurveySentAt = Date.now()
followup.smsSurveyMessageId = data.messageIds[0]
followup.smsSurveyError = undefined
Copy link
Contributor Author

@Shamzic Shamzic Nov 17, 2023

Choose a reason for hiding this comment

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

Preneur d'avis concernant le stockage de ces données. Ces assignations similaires à celles effectuées dans la partie d'envoi par email ne me semblent pas forcément utiles. Je les ai ajoutées dans le doute car je n'ai pas de spécification à ce sujet

@Shamzic Shamzic force-pushed the feature-envoi-survey-sms branch from eb22f1c to 9803602 Compare November 17, 2023 17:25
@Shamzic Shamzic force-pushed the feature-envoi-survey-sms branch from 8b3dc7b to e5b911f Compare November 17, 2023 18:08
@Shamzic Shamzic changed the title Rend possible l'envoi du lien d'accès au sondage par SMS Envoi du lien du sondage par SMS Nov 17, 2023
@Shamzic
Copy link
Contributor Author

Shamzic commented Nov 21, 2023

Résolution des multiples rebase disponible ici dans une nouvelle PR (plus simple à review)

@Shamzic Shamzic closed this Nov 21, 2023
@guillett guillett added this to the BC milestone Nov 24, 2023
@Shamzic Shamzic deleted the feature-envoi-survey-sms branch February 8, 2024 10:05
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.

3 participants