-
Notifications
You must be signed in to change notification settings - Fork 30
PR – Happy Thoughts API - Juan Z. #15
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
base: master
Are you sure you want to change the base?
Conversation
…middleware for user authentication, migrate to jwt
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, Juan! Your project is super clean, with clear separation of routes and controllers, consistent error handling, and solid use of Mongoose models. I especially appreciate the thorough input validation and JWT‐based auth flows. everything seems to integrate nicely.
One last recommendation before wrapping up: in your controllers/postUser.js and controllers/loginUser.js, make sure you’re hashing passwords before saving them (for example with bcrypt in a pre-save hook or virtual setter) and comparing hashes on login. That will lock down user credentials and fully satisfy the requirement to encrypt passwords in the database.
Great job!
import mongoose from "mongoose" | ||
import dotenv from "dotenv" | ||
|
||
import { getThoughts } from "./controllers/getThoughts.js" |
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.
Consider grouping your thought routes under a router (e.g. app.use('/thoughts', thoughtsRouter)) rather than mounting each individually. It keeps server.js cleaner and matches the pattern we saw in one of the live sessions for the flower project.
app.get("/thoughts", authenticateUser, getThoughts) | ||
|
||
// Endpoint for getting a specific thought by id | ||
app.get("/thoughts/:id", authenticateUser, getThought) |
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 interpret the requirements for “Reading thoughts” (i.e. listing & getting) should work for anyone, even not logged in. I’d recommend removing authenticateUser from these two so that a public user can browse thoughts without a token.
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 job with this project!
I'm testing your app from your frontend some maybe there are frontend bugs or backend bugs. I found two issues...
When liking, it seems that I can only like some of the thoughts. Not necessarily my own but also from others. See the network tab
Another thing is that all thoughts shows the same timestamp,
Take a look at those things and ping me when it's fixed.
All in all, very good structured code. Easy to follow and clean approach.
Link to API on Render: https://js-project-api-4xto.onrender.com/
Link to Frontend on Netlify: https://happy-thoughts-react.netlify.app/