-
Notifications
You must be signed in to change notification settings - Fork 516
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
Project Happy Thoughts API - Kathinka #487
base: master
Are you sure you want to change the base?
Conversation
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 job Kathinka, cool to see that you played around a little with the different methods and possibilities ⭐ 🥳
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.
File names should be kebab-case (lowercase) if nothing else is specified 😇
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.
But nice idea breaking this out 👍
model/Happythoughts.js
Outdated
|
||
const HappyThought = mongoose.model("HappyThought", happyThoughtSchema); | ||
|
||
export default HappyThought; |
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.
It's good practice to always leave an empty line at the end of JS files
routes/Routes.js
Outdated
@@ -0,0 +1,159 @@ | |||
import express, { response } from "express"; |
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.
Why import { response } here? 👀
.sort({ createdAt: "desc" }) | ||
.limit(20) | ||
.exec(); | ||
res.json(happyThoughts); |
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.
A status code could be nice here
const newHappyThought = await new HappyThought({ message }).save(); | ||
res.status(201).json(newHappyThought); | ||
} catch (error) { | ||
console.error(error); // Log the error |
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.
Remove console.logs in production
const updatedThought = await HappyThought.findByIdAndUpdate( | ||
thoughtId, | ||
{ $inc: { hearts: 1 } }, | ||
{ new: true } | ||
); |
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.
👍
{ new: true } | ||
); | ||
if (updatedThought) { | ||
res.json(updatedThought); |
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.
Status code could be nice here too
}); | ||
|
||
//delete a happy thought | ||
router.delete("/thoughts/:thoughtId", async (req, res) => { |
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! Would be cool if it was reflected in your frontend as well, but since auth is not until this week, I understand you didn't want anyone to be able to delete anyone else's posts
}); | ||
|
||
//filter happy thoughts by limit or skip or both | ||
router.get("/thoughts/q", async (req, res) => { |
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.
This doesn't need to be its own endpoint, you can include the query in your /thoughts endpoint
}); | ||
|
||
// get updated documentation | ||
router.get("/", (req, res) => { |
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.
Most generic endpoints should always come first, e.g.
- /
- /books
- /books/:id
Netlify link
API
Site
Collaborators
solo