Skip to content
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.

Sign up for GitHub

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

NW6 | Zeliha Pala | Full-Stack-Project | Week-2 | DELETE-videos-endpoint #25

Merged
merged 5 commits into from
May 26, 2024
Merged

NW6 | Zeliha Pala | Full-Stack-Project | Week-2 | DELETE-videos-endpoint #25

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3d7ac5b
Add DELETE /api/videos/:id route
zelihapala May 16, 2024
676ab6e
updated
zelihapala May 25, 2024
2b59f10
post videos bug related to database (rating column configuration whic…
zelihapala May 25, 2024
574e88d
?? operator added for rating value to post route
zelihapala May 25, 2024
9f3e408
Merge branch 'main' of https://github.com/RbAvci/Full-Stack-Project-A…
zelihapala May 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions server/api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,29 @@
res.status(200).json({ success: true, data: { id: newVideoId } });
});

router.delete("/videos/:id", async (req, res) => {
const videoId = req.params.id;

try {
const checkQuery = await db.query("SELECT * FROM videos WHERE id = $1", [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

`${videoId}`,
]);
if (checkQuery.rows.length === 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work 👏

return res.status(404).json({
message: "Video not found",
});
}

const deleteQuery = await db.query("DELETE FROM videos WHERE id = $1", [

Check failure on line 43 in server/api.js

View workflow job for this annotation

GitHub Actions / run-linter 

'deleteQuery' is assigned a value but never used
videoId,
]);

return res.status(204).end();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would normally return 200 here. 204 is very rarely used so users might think its an error

} catch (error) {
res
.status(500)
.json({ message: "Internal server error", error: this.error });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your reviews I have changed some parts of my codes according to your recommendations.

}
});

export default router;
Loading