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

ETQ utilisateur, le form de contact détecte les typos d'email et valide les champs avant de l'envoyer à HS #10644

Merged

Conversation

colinux
Copy link
Member

@colinux colinux commented Jul 24, 2024

Contexte :

  • des utilisateurs (non connectés) soumettent le form de contact avec une adresse mail erronée (type hardbounce comme @gmail.con) 19K occurrences en 30j avec les retries, dont des bots qui polluent sentry
  • on leur affiche "OK message envoyé" alors qu'en fait, il sera rejetté dans le job par HS et on le recevra jamais
  • avant le passage en async le mois dernier, HS renvoyait l'erreur en direct, ce qui faisait pas croire que le message était envoyé

Cette PR permet de limiter ces cas là en :

  • utilisant le composant de suggestion/correction d'email
  • validant les emails strict car c'est ce que semble faire HS

Pour ça le form est modélisé, ce qui implique pas de mal refacto et simplifications, et permet de corriger des pb sur le form de contact admin qui était encore sur l'ancien design

Note: certains seront probablement tjs bloqués car on valide pas les hardbounce de notre côté

Contact admin APRES

Capture d’écran 2024-07-24 à 18 09 43

AVANT

Capture d’écran 2024-07-24 à 18 09 03

Copy link

sentry-io bot commented Jul 24, 2024

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: app/controllers/support_controller.rb

Function Unhandled Issue
create [**NoMethodError: undefined method tempfile' for an instance of String (NoMethodError)**](https://demarches-simplifiees.sentry.io/issues/5535577424/?referrer=github-open-pr-bot) ... <br> Event Count:` 464

Did you find this useful? React with a 👍 or 👎

@colinux colinux force-pushed the fix-helpscout-invalid-email branch 2 times, most recently from c0e8387 to b8e89d9 Compare July 24, 2024 16:39
Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 97.22222% with 3 lines in your changes missing coverage. Please review.

Project coverage is 80.22%. Comparing base (90e6c8a) to head (e415c79).
Report is 4 commits behind head on main.

Files Patch % Lines
app/controllers/contact_controller.rb 95.23% 2 Missing ⚠️
app/jobs/helpscout_create_conversation_job.rb 95.23% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10644      +/-   ##
==========================================
+ Coverage   80.20%   80.22%   +0.02%     
==========================================
  Files        1241     1241              
  Lines       26415    26428      +13     
  Branches     4737     4741       +4     
==========================================
+ Hits        21185    21202      +17     
+ Misses       5230     5226       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@colinux colinux force-pushed the fix-helpscout-invalid-email branch 2 times, most recently from e046e74 to 00efd8e Compare July 24, 2024 21:13
Copy link
Member

@LeSim LeSim left a comment

Choose a reason for hiding this comment

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

Coucou, des ptites remarques, rien d'importants. Donc pour le principe gogo.

Après, je me demande si ca vaudrait pas le coup de créer carrément un ActiveRecord::Model pour Helpscout::Form.

Ca permettrait de pas s’embêter à manipuler l'attachment a la main, de filer un id à HelpscoutCreateConversationJob.perform_later

finalement, on pourrait (ou pas) déléguer à cette object create_converstation

par ailleurs, et en bonus, si tu peux nous rajouter un commit supplémentaire qui déplace le create_commentaire de la ligne 13 sous le if, en le renommant en create_commentaire! car il fait un create! et ne renvoie pas de booléen, c'est top !

app/controllers/support_controller.rb Outdated Show resolved Hide resolved
app/controllers/support_controller.rb Outdated Show resolved Hide resolved
@colinux colinux marked this pull request as draft July 30, 2024 08:08
@colinux colinux force-pushed the fix-helpscout-invalid-email branch 3 times, most recently from 87289ca to 9b94352 Compare July 30, 2024 18:59
@colinux colinux force-pushed the fix-helpscout-invalid-email branch 4 times, most recently from c0f435a to 66322d4 Compare July 31, 2024 10:14
@colinux colinux marked this pull request as ready for review July 31, 2024 10:39
Copy link
Member

@LeSim LeSim left a comment

Choose a reason for hiding this comment

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

Allez hop, ca roule !

Des ptites remarques, mais rien d'important

app/controllers/support_controller.rb Outdated Show resolved Hide resolved
app/controllers/support_controller.rb Outdated Show resolved Hide resolved
app/controllers/support_controller.rb Outdated Show resolved Hide resolved
@colinux colinux force-pushed the fix-helpscout-invalid-email branch 2 times, most recently from 261f13f to ee47a71 Compare July 31, 2024 14:54
@colinux colinux force-pushed the fix-helpscout-invalid-email branch from ee47a71 to e415c79 Compare July 31, 2024 15:08
@colinux colinux changed the title ETQ utilisateur, le form de contact suggère et valide l'email avant de l'envoyer à HS ETQ utilisateur, le form de contact détect les typos d'email et valide les champs avant de l'envoyer à HS Jul 31, 2024
@colinux colinux changed the title ETQ utilisateur, le form de contact détect les typos d'email et valide les champs avant de l'envoyer à HS ETQ utilisateur, le form de contact détecte les typos d'email et valide les champs avant de l'envoyer à HS Jul 31, 2024
@colinux colinux added this pull request to the merge queue Jul 31, 2024
github-merge-queue bot pushed a commit that referenced this pull request Jul 31, 2024
ETQ utilisateur, le form de contact détecte les typos d'email et valide les champs avant de l'envoyer à HS
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 31, 2024
@colinux colinux added this pull request to the merge queue Jul 31, 2024
Merged via the queue into demarches-simplifiees:main with commit 3b82621 Jul 31, 2024
19 checks passed
@colinux colinux deleted the fix-helpscout-invalid-email branch July 31, 2024 17:09
Copy link

sentry-io bot commented Aug 1, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ **NoMethodError: undefined method piece_jointe' for an instance of Hash (NoMethodError)** Sidekiq/HelpscoutCreateConversationJob` View Issue
  • ‼️ RuntimeError: Error while creating conversation: 400 '{"logRef":"6e993505-671f-4cc4-9b4d-843016e8624d#20628937"... Sidekiq/HelpscoutCreateConversationJob View Issue
  • ‼️ ActiveRecord::DeleteRestrictionError: Cannot delete record because of dependent dossiers (ActiveRecord::DeleteRestrictionError) Sidekiq/HelpscoutCreateConversationJob View Issue
  • ‼️ ActiveSupport::MessageVerifier::InvalidSignature: ActiveSupport::MessageVerifier::InvalidSignature (ActiveSupport::MessageVerifier::InvalidSignature) ContactController#create View Issue
  • ‼️ ActiveModel::RangeError: 9761202402200198642 is out of range for ActiveModel::Type::Integer with limit 8 bytes (ActiveMode... ContactController#create View Issue

Did you find this useful? React with a 👍 or 👎

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