-
Notifications
You must be signed in to change notification settings - Fork 30
Happy thoughts - Tilde #12
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
Loading thoughts from data.json
Routes - get, post, patch, delete
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.
Hi Tilde! As always great work! I tried to find stuff to improve on but you make that hard with you great work. I think you had a good structure, good naming and clean code. Makes it easy to follow along. Especially loved the routes that you split up. The only thing I found that could be approved upon is that you could make the const in your routs their own files so that the file does not get to large. But maybe that is just my preference.
But once again, super great work as always. Really look forward to see your and Idas final project!
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 to do list! I need to start getting more organised like this even when working solo!
} | ||
}; | ||
|
||
// PATCH /thoughts/:id/like |
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 comments so that it is easy to look up!
// PATCH /thoughts/:id/like | ||
export const likeThought = async (req, res) => { | ||
try { | ||
const updated = await Thought.findByIdAndUpdate( |
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 am wondering myself. Is update a good name? At first a thought no but then when I thought about it that it is in a function maybe it is overkill to have like updateThought or whatever. So I think it is a good name!
{ $inc: { hearts: 1 } }, | ||
{ new: true } | ||
); | ||
if (!updated) return res.status(404).json({ message: "Thought not found" }); |
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 error handling! Like all the rest you did!
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 great! Only thing that could be nice is to add the different consts into a different util component to make it easier to find the right one. Sort of like you did in your routes file.
// DELETE /thoughts/:id – Delete a thought | ||
router.delete("/:id", authenticateUser, deleteThought); | ||
|
||
export default router; |
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.
Do you like export default more? I prefer the export const. Nothing wrong just different ways of doing it
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.
Good error handeling and clear messages
|
||
app.use("/thoughts", thoughtsRoutes); | ||
|
||
app.use("/users", usersRoutes); |
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 like once again that you broke out your routes for a cleaner code
import fs from "fs"; | ||
|
||
const loadThoughts = () => { | ||
const data = fs.readFileSync("data/data.json"); |
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.
What is: fs.readFileSync
|
||
const mongoUrl = mongoose | ||
.connect(mongoUrl, { useNewUrlParser: true, useUnifiedTopology: true }) | ||
.then(() => console.log("🟢 Connected to MongoDB")) |
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.
Love the emojis for a clearer message
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.
loadThoughts and seedThoughts? what was the idea with both of them? I guess you only want to seed the db once with that json and then use the data from your db?
Well structured project, clear separation and consistent formattin, naming etc.
When trying to register it seems to be stuck?
Can you take a look and ping me when it's working so I can continue with the review?
@@ -0,0 +1,53 @@ | |||
# ✅ TODO: Connect Frontend & Backend for Happy Thoughts |
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 way to organise your work.
Hi Jennie,
Thank you for this!
It works for me in different browsers. Could you let me know which
browser/version you used and the steps you took? That will help me
reproduce it.
Kind regards,
Tilde
…On Thu, Aug 21, 2025 at 9:59 AM Jennie Dalgren ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Great job with this project.
loadThoughts and seedThoughts? what was the idea with both of them? I
guess you only want to seed the db once with that json and then use the
data from your db?
Well structured project, clear separation and consistent formattin, naming
etc.
When trying to register it seems to be stuck?
image.png (view on web)
<https://github.com/user-attachments/assets/73d39018-48b7-443c-98bc-efeb71aed83a>
Can you take a look and ping me when it's working so I can continue with
the review?
------------------------------
In TODO.md
<#12 (comment)>
:
> @@ -0,0 +1,53 @@
+# ✅ TODO: Connect Frontend & Backend for Happy Thoughts
⭐ Great way to organise your work.
—
Reply to this email directly, view it on GitHub
<#12 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BBT6BW7AFBS2HUM6OVS2KYT3OV36XAVCNFSM6AAAAAB7LSXWK2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTCMZZGQ3TCNBXGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Render:
https://happy-thoughts-api-5hw3.onrender.com
Netlify:
https://happyping.netlify.app/
Frontend:
https://github.com/tildetilde/js-project-happy-thoughts