-
Notifications
You must be signed in to change notification settings - Fork 0
Allows load testing for faxes #177
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
base: master
Are you sure you want to change the base?
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/votebymail/build/kbekmn3bg |
262ed98 to
d4477d9
Compare
d4477d9 to
fb35e80
Compare
tianhuil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small tweaks
| @@ -1,3 +1,6 @@ | |||
| // To test this script for faxes run it using --faxes on the CLI, or by | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to send actual emails or faxes during the test. Is there a way to prevent this or to mock what gets sent?
| const letter = new Letter(info, method, id, resendReasonAndOriginalDate) | ||
| const pdfBuffer = await toPdfBuffer(letter.render('html'), await letter.form) | ||
| const isResend = resendReasonAndOriginalDate !== undefined | ||
| // When performing signups for DIVERT_EMAIL we don't send emails or faxes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong -- we use DIVERT_FAX_NUMBER and DIVERT_EMAIL separately. Let's stick to using that logic.
I don't think we should be editing signup.ts very much for this.
No description provided.