Skip to content
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

added contactRequests #21

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

added contactRequests #21

wants to merge 7 commits into from

Conversation

westonz7042
Copy link

Resolves #4

Added POST api/contact route, as well as handler and validator for contact requests. Created error file for contacts and updated backend .env with testing email + app password for nodemailer.

Tested with sample contact request in postman - console logged successfully and email was sent by the homework email to the homework email (as intended).

image
image

@westonz7042 westonz7042 requested a review from mraysu as a code owner January 11, 2025 23:45
@mraysu
Copy link
Collaborator

mraysu commented Jan 12, 2025

I came across a few issues. I would prefer that each of our request handlers can propagate to the next function to effectively leverage the error handler. Please add a next parameter to the handleContactRequest function.

Also, there's no need to create a separate promise called sendContactEmail, then calling the promise. Along with being poor coding practice, this ends up being an issue in this implementation, as failed emailing attempts still achieve a 200 status code, which is a bug.
image
image

Also can you make sure to add the app password since I got the error

@mraysu mraysu requested review from PoliteUnicorn and SaazM January 13, 2025 23:28
@westonz7042
Copy link
Author

Updated .env variables to include email and email app password for contact request

@mraysu mraysu removed the request for review from SaazM January 15, 2025 23:35
Copy link
Collaborator

@mraysu mraysu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

mraysu
mraysu previously approved these changes Jan 16, 2025
PoliteUnicorn
PoliteUnicorn previously approved these changes Jan 16, 2025
Copy link

@PoliteUnicorn PoliteUnicorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@westonz7042 westonz7042 dismissed stale reviews from PoliteUnicorn and mraysu via cf835be January 16, 2025 02:38
Copy link
Collaborator

@mraysu mraysu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unable to compile. Please remove the export from the removed errors/contact.ts file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Contact Request Backend Route
3 participants