-
Notifications
You must be signed in to change notification settings - Fork 0
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
Wrapped emailing #913
base: master
Are you sure you want to change the base?
Wrapped emailing #913
Conversation
[diff-counting] Significant lines: 305. |
- also tested that batching still sends email to multiple people in the array
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.
Nice job testing this thoroughly and documenting your process well
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.
Great work with this and figuring out the Resend API! No glaring issues I can see with this – only thing to watch out for when we run it is if we hit a limit, what's the best way to continue where we left off (if manually doing it is the best option, that's ok too).
src/scripts/email/wrapped-email.ts
Outdated
if (batchSize > 49) { | ||
throw new Error("Batch size is too large. Must be no more than 49"); | ||
} | ||
while (i < totalEmails.length && emailObjs.length <= 100) { |
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.
maybe add an extra check totalEmails.length > 4900
in case
src/scripts/email/wrapped-email.js
Outdated
} | ||
if (emailObjs.length === 100) { | ||
// eslint-disable-next-line no-console | ||
console.log("Reached email limit of 100 emails per day, stopped at i=" + i + ", user " + totalEmails[i] + ". \n Continue from this user the next day."); |
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.
how would we continue from this user the next day? are we manually keeping track of the last index?
// using orderBy for email field to filter out users that don't have an email | ||
const usersSnapshot = await usersRef | ||
.where('wrapped', '==',true) | ||
.orderBy('email') |
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.
add a .where('email', '!=', null)
condition to ensure we only fetch users with a valid email address (i think it's not always the case that a user's email exists)
3de7139
to
b3674b2
Compare
Summary
Credit to CoursePlan's email system for instructions to set up the system.
This PR creates a script that uses the Resend API to send automated emails to users that can see Wrapped. At most, it can send 100 emails each as a batch with 49 recipients in a day.
If it's the first time running the script, you should run:
node <script path> 0
This will start the script from the first email (0th index). If the 100 email limit has been hit, there will be a console statement that tells you the next input number to run the next day.
Test Plan
Laptop:
Mobile:
Notes
In the prod database would need to also update indexes for user like below:
Firebase auto-generates this when you run the script and get an initial error message with a link:
Should look like this at the end and script should be working
Created online references to images with ImgBB
Fonts like Roboto don't seem to be rendering properly across all email providers - might just not be easily feasible
Breaking Changes
None