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

Fix invalid email error in process_forms #19602

Merged
merged 1 commit into from
Dec 13, 2017
Merged

Conversation

jeremydstone
Copy link

Since forever, we have occasionally had emails get submitted in forms that are later judged invalid by Poste. When Poste throws an exception about an invalid email, that completely stops our process_forms cron job and requires manual intervention to repair.

This PR makes process_forms more robust so the entire job does not fail due to a single invalid email address. process_forms now rescues from an "Invalid email address" exception from Poste and continues, marking the form as processed. (The same thing we do to repair manually.)

This problem (though infrequent) occurs because we have different (and less stringent) email validation rules in place elsewhere on the site. We should consider tracking these down and making Poste and the rest of our site use the same validation rules. This fix is worth having even if we align the email validation as well, since the present behavior of one invalid entry making the whole process fail is not robust.

@aoby
Copy link
Contributor

aoby commented Dec 13, 2017

Thanks for fixing!

Let's add the follow up to our tech debt tracking doc, so we don't lose track of it.

@jeremydstone
Copy link
Author

Also: tested locally by running process_forms against some local data with valid and invalid emails set and made sure the changes behaved as expected.

@jeremydstone jeremydstone merged commit 3d4296f into staging Dec 13, 2017
@jeremydstone jeremydstone deleted the fix_invalid_email_error branch December 13, 2017 00:33
@aoby aoby mentioned this pull request Jan 10, 2018
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.

None yet

2 participants