search

freeCodeCamp / pantry-for-good

An open source food bank logistics and inventory management tool
Other
395 stars 189 forks source link

(#157) add a 'you signed up with your google account' email #343

Closed ronaldblanco closed 6 years ago

ronaldblanco commented 6 years ago

Closes the Issue #157 add a 'you signed up with your google account' email. All changes were locally tested and lint checked.

jspaine commented 6 years ago

Hi, it looks good! This one was a bit complicated though because I'm not sure the email needs to be editable, it should just say something like 'you appear to have signed up with google, try logging in with google. if you have any problems send us a message at support@foodbank.email' and can just have default translations (when we get to that). Or what do you think?

Another part of this issue was that the server users password controller should just send a 200 regardless, and the client should show 'an email has been sent to email@address', so the route can't be used to figure out registered users email addresses. You don't have to do this too, I was a bit late for the discussion on the issue but thought it was worth mentioning.

ronaldblanco commented 6 years ago

Hello @jspaine; I do not understand what do you mean with: "can just have default translations (when we get to that). Or what do you think?" I did search in the other emails and I do not see any translations options implemented. It is something new do you want me to code?

jspaine commented 6 years ago

Hi, I just meant that this email is so simple it's not really worth being editable, and there could just be a standard one. But it makes the code a bit more complicated to make it not editable. I guess we shouldn't worry about it for now and leave it editable.

I'll leave some suggestions in the code, but the context of this is a user enters their email in the forgot password form but they signed up with google so don't have a password and the password reset email wouldn't be appropriate. So I don't think the place you're sending the email from is right, look for how the forgot password form is handled on the server.

ronaldblanco commented 6 years ago

Hey @jspaine take a look at this.

ronaldblanco commented 6 years ago

Check now. When you think everything it is right I will squash.

ronaldblanco commented 6 years ago

Check now, if it is ok I will clean the code and squash.

ronaldblanco commented 6 years ago

Check now!

jspaine commented 6 years ago

Try to think through it more, the if...else's can be made a bit simpler now. We only want to send an email if there's a user.

mailer.sendPasswordReset needs the password reset url too.

ronaldblanco commented 6 years ago

And now??

jspaine commented 6 years ago

Yeah that looks good.

ronaldblanco commented 6 years ago

It should be ready! Let me know.

jspaine commented 6 years ago

Thanks, looks good!