Send email #188
Send email #188
Conversation
|
will running the tests send real emails out? EDIT: I realised It will only be sending to fake addresses but still doesn't seem like a good idea |
src/helpers/sendEmail.js
Outdated
|
|
||
| module.exports = content => | ||
| new Promise((resolve, reject) => | ||
| User.find() |
There was a problem hiding this comment.
you can put parameters in the find method to filter only the users you want, and you can also select the fields you want (just email or maybe name and email). Then you won't need the extractEmails call
There was a problem hiding this comment.
huh nice! didn't know this before
src/helpers/sendEmail.js
Outdated
| }, []) | ||
|
|
||
| module.exports = content => | ||
| new Promise((resolve, reject) => |
There was a problem hiding this comment.
you don't need the new Promise(...) part, you can do content => User.find(...).then(...)
|
@mattlub yeah it's currently sending emails to |
| @@ -50,7 +51,7 @@ eventController.create = (req, res, next) => { | |||
|
|
|||
| const newEvent = new Event(newEventDetails) | |||
| newEvent.save() | |||
| // TODO: .then(event => req.user.role !== roles.BASIC ? event : sendEmail(event)) | |||
| .then(event => req.user.role !== roles.BASIC ? event : sendEmail(event)) | |||
There was a problem hiding this comment.
Do not put side effects in a ternary! (use an if / else)
Your async flow is cray cray, does sendEmail return a promise?
There was a problem hiding this comment.
i don't see any problem here other than readability maybe?, if i had this in if/else and in ternary it's just going to do the same thing if the user is BASIC then sends an email then the sendEmail function would resolve with the event, if he's not then resolve with the event without sending the email. so whats the reason here to put this in an if statement instead of ternary ?! @des-des
| @@ -35,6 +36,7 @@ placeController.create = (req, res, next) => { | |||
|
|
|||
| const newPlace = new Place(newPlaceDetails) | |||
| newPlace.save() | |||
| .then(place => req.user.role !== roles.BASIC ? place : sendEmail(place)) | |||
| const sendEmails = (emails, content) => { | ||
| const msg = { | ||
| to: emails, | ||
| from: '[email protected]', |
close #187
I have made an account on sendGrid so i would build the template on their website with HTML and CSS
currentley doesn't look great but it has the name of the content and a link of the content in the data entry ( also arabic support 😉 ) :
this is how literately it's gonna look like in the email
this is how it looks like when you recieve it :
also right now everyone who is not a
BASICuser is sent an email, should we keep it like that or should i make itADMINrecieve emails only ?