-
Notifications
You must be signed in to change notification settings - Fork 31
API-backend-BiankaR #18
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
Lillebrorgroda
left a comment
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, Bianka! This is a well-structured RESTful API with clean and readable code. You’ve successfully met all the key requirements from what I can see. Your models, authentication flow, and route handling are solid and follow best practices. Error handling is clear, and the code is easy to follow and maintain. Keep up the great work!
| email: { | ||
| type: String, | ||
| unique: true, | ||
| sparse: true, | ||
| lowercase: true, | ||
| trim: true, | ||
| validate: { | ||
| validator: (email) => { | ||
| return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email) | ||
| }, | ||
| message: props => `${props.value} is not a valid email address!` | ||
| } |
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.
Nice use of a custom message and validator for the email field!
I'm curious about your use of sparse — did you include it intentionally? If so, that’s a clever way to allow the email to be unique without making it required. I like how you're thinking about flexibility in your data model!
| "bcrypt-nodejs": "^0.0.3", | ||
| "bcryptjs": "^3.0.2", |
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.
Looks like you're only using bcryptjs — maybe remove bcrypt-nodejs to keep things clean?
| @@ -0,0 +1,119 @@ | |||
| import bcrypt from "bcryptjs" | |||
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 structure and very clear authentication!
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.
Nicely structured routes with great error handling!
JennieDalgren
left a comment
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 like how you have made a really neat implementation of your backend all the way to the front as well.
You code is well structured and easy to follow. You are working well with error handling, authentication and validation. Nice work with the regular expression.
Keep up the good work, looking forward to see your final project!
| @@ -0,0 +1,114 @@ | |||
| [ | |||
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.
If the db is seeded you dont really need to keep this json. It's okay if you want to keep it but not really necessary
| validate: { | ||
| validator: (email) => { | ||
| return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email) | ||
| }, |
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.
⭐
Please include your Render link here: https://js-project-api-b8sd.onrender.com
Frontend deployed site: https://happy-thoughts-blr.netlify.app/
Frontend code/PR: Technigo/js-project-happy-thoughts#27