-
Notifications
You must be signed in to change notification settings - Fork 37
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
BL-751-BE-Build an Email Service #124
base: main
Are you sure you want to change the base?
Conversation
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.
@paulstgermain I mentioned to Lisa that we'd be able to grant credit for this work. Even though it won't make it's way into the codebase, this is how I see Spikes playing out in the future via open Draft PR. There are ways to clean up the code here, but I'm more concerned about using this work as a learning platform as we move to rewrite our Backend in Java. @ashtilawat23 take note of the implementation here as it'll be similar in Java but using the Spring/Boot tutorial on Sendgrid's documentation site.
@ryan-hamblin Absolutely, that makes sense to me! I'll make sure credit is given for this work now, and I'm sure our Java engineers will be thankful for the leg up this provides! |
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 really amazing work. I know a lot of time and effort was put into this, and it really shows. I've left a few minor suggestions in my comments, but overall I am very impressed with what you've accomplished here.
.gitignore
Outdated
@@ -39,3 +39,6 @@ jspm_packages/ | |||
.env | |||
.env.test | |||
.env.local | |||
sendgrid.env | |||
sendgrid.env | |||
sendgrid.env |
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.
Should there be three lines with sedgrid.env here?
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.
Nope, removed 2 of them.
README.md
Outdated
@@ -300,3 +299,47 @@ Visual Database Schema: https://dbdesigner.page.link/WTZRbVeTR7EzLvs86 <b>\*Curr | |||
[Loom Video PT4](https://www.loom.com/share/7da5fc043d3149afb05876c28df9bd3d) | |||
|
|||
<br /> | |||
|
|||
<h2>Email Service</h2> | |||
In api/email, the emailHelper.js file contains the following functions: sendEmail and addToList. SendEmail sends an API request to SendGrid to send a templated email to the to email address its given. The other function: addToList adds the email and name to a specified SendGrid contact list (they're added to all no matter what, then also to the specified list id). Any function that wants to use sendEmail and addToList will need to import it into their file (see profileRouter.js). Parameters can be passed into it, like the toEmail, name, template_id or list_ids (which is an array so it could have more than 1 list_ids there). The parameters to use are created in the file that's calling the sendEmail function. |
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.
Slight typo in the second sentence of this paragraph "SendEmail sends an API request to SendGrid to send a templated email to the to email address it's given." Just have an extra to in there.
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.
Got it - thanks!
<h2>Email Service</h2> | ||
In api/email, the emailHelper.js file contains the following functions: sendEmail and addToList. SendEmail sends an API request to SendGrid to send a templated email to the to email address its given. The other function: addToList adds the email and name to a specified SendGrid contact list (they're added to all no matter what, then also to the specified list id). Any function that wants to use sendEmail and addToList will need to import it into their file (see profileRouter.js). Parameters can be passed into it, like the toEmail, name, template_id or list_ids (which is an array so it could have more than 1 list_ids there). The parameters to use are created in the file that's calling the sendEmail function. | ||
|
||
SendGrid's npm package info and how to set up the API key (also listed below): https://www.npmjs.com/package/@sendgrid/mail. The npm package is @sendgrid/mail. Very lightweight, highly supported, and heavily downloaded. (SendGrid also supports Java) |
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.
Awesome! This is great documentation for people working on this in the future. I am sure they will appreciate how thorough you are.
|
||
AddToList: First up , add list(s) to the SendGrid account. In addition to the default "all" list, we added instructors, parents, and students. From a marketing perspective, these are the major groups, each requiring a different kind of information, and will potentially need different mass email types. | ||
|
||
As part of the request in emailHelper.js, you'll send the id(s) of the contact list (list_ids, an array) you want to add the email to, plus any additional data you want to add to the request, like the email address (as 'email') and name (as 'first_name'). If you use custom fields, make sure the fields are already set up in SendGrid or SG won't know where to map them. |
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 fantastic. You've done an excellent job explaining how the process works. I feel like I could set up SendGrid after reading these instructions. Fantastic work.
__tests__/routes/emailHelper.test.js
Outdated
name: 'New Student Here', | ||
}, | ||
to: '[email protected]', | ||
from: '[email protected]', // verified sender in SendGrid account |
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.
I am not sure what the comment here is trying to tell me. It may be beneficial to flesh it out a little to make things more clear, or remove it all together if it isn't necessary
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.
Made more clear, also explained domain validation.
api/email/emailHelper.js
Outdated
sgMail | ||
.send(data) | ||
.then((response) => { | ||
// note: the following 2 console logs are SendGrid out of the box. Keep them if you like them. We found the stringify response to be more descriptive. |
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.
I think I would remove the commented out console.logs. If you feel the stringify response is better, then that's what should be used.
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.
That makes sense. I removed them.
console.error(e); | ||
res.status(500).json({ message: e.message }); | ||
const profileExists = await Profiles.findById(profile.okta_id); | ||
if (profileExists) { |
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.
Would it be better to put this check for whether the profile exists in the checkProfileObject middleware?
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.
Probably yes, but I didn't want to break existing functionality.
api/profile/profileRouter.js
Outdated
if (profileExists) { | ||
res.status(400).json({ message: 'profile already exists' }); | ||
} else { | ||
const prof = await Profiles.create(profile); |
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.
To me, the variable name "prof" seems a little non-descript and a bit too similar to "profile" used above. I might consider renaming this to "newProfile".
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.
I like that - will do.
Description
Owner: Lisa DeSpain, Co-owners: Renee Troiani and Colya Foxworth
The goal was to create an email service using SendGrid's API. First, we had to determine the stakeholder's needs. When would an email need to automatically go out? How could we use the backend to enable automatic email delivery, with customization? We decided to use the email service to send automated emails when the profile was created to different user types: instructors, parents, and students. Within profileRouter.js post function, we used conditional logic to send a request to the sendEmail function in the emailHelper.js file. We used a different SendGrid template for each of the types of users. Part 2 of this was to add that new email and contact name to a contact list on SendGrid. We did that in profileRouter.js by passing the list_ids to the addToList function in emailHelper.js. Finally, we updated the README file with a roadmap for devs on the Java team.
##Loom:
Loom walkthrough on backend: https://www.loom.com/share/a7d867ee6f9f4bca9a4a3e911bca3de4
Loom how to see if it's working: https://www.loom.com/share/7acaa082c8454ac4bf17e0670aec18d7
Type of change
Please delete options that are not relevant.
Checklist: