NW6 | Zeliha Pala | Full-Stack-Project | Week-2 | DELETE-videos-endpoint #25
Conversation
zelihapala
commented
May 16, 2024
zelihapala
commented
- Handle non-existent IDs with 404
- Return 204 on successful deletion
- Use parameterized queries
- Consistent error handling
- Handle non-existent IDs with 404 - Return 204 on successful deletion - Use parameterized queries - Consistent error handling
✅ Deploy Preview for watch-next-cyf ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
zelihapala
changed the title
| const videoId = req.params.id; | ||
|
|
||
| try { | ||
| const checkQuery = await db.query("SELECT * FROM videos WHERE id = $1", [ |
There was a problem hiding this comment.
It's good to see you preventing SQL Injection by using parameters! 🙌 If you’re not sure what I'm on about, Google SQL Injection. @RbAvci This is a good example of preventing SQL Injection.
| const checkQuery = await db.query("SELECT * FROM videos WHERE id = $1", [ | ||
| `${videoId}`, | ||
| ]); | ||
| if (checkQuery.rows.length === 0) { |
| videoId, | ||
| ]); | ||
|
|
||
| return res.status(204).end(); |
There was a problem hiding this comment.
I would normally return 200 here. 204 is very rarely used so users might think its an error
server/api.js
Outdated
| } catch (error) { | ||
| res | ||
| .status(500) | ||
| .json({ message: "Internal server error", error: this.error }); |
There was a problem hiding this comment.
Good to see you're sending the error back rather than a boilerplate message. In production proper however, it's worth thinking about what parts of the error object the user actually needs and sending those bits back so that there is only the info they need. No need for this project
There was a problem hiding this comment.
Thank you for your reviews I have changed some parts of my codes according to your recommendations.
zelihapala
changed the title
…h is "not null") fixed, ratings added to post endpoint
…ssessment into ZP/delete-videos-endpoints
