London_10 | Yulia - Hospodar | Full-Stack-Project | #447
Conversation
illicitonion
left a comment
illicitonion
left a comment
There was a problem hiding this comment.
This is generally looking really good! I've left a few comments of things to think about / improve :)
|
|
||
| const SingleVideo = ({ video, handleDeleteVideoClick }) => { | ||
| const { title, url, rating, id } = video; | ||
| const [votes, setVotes] = useState(0); // can be 0, 1 or -1 => to add to rating when thumbsUp or ThumbsDown is clicked |
There was a problem hiding this comment.
It's really good to include a comment explaining what values are allowed/possible when ony some values are allowed - good job!
There was a problem hiding this comment.
That said, I don't see any code that prevents votes from being set to 2, or -2 - do you intend for there to be code stopping that?
| className="responsive-iframe" | ||
| width="560" | ||
| height="310" | ||
| src={`https://www.youtube.com/embed/${url.split("v=")[1]}`} |
There was a problem hiding this comment.
What would happen if a URL didn't contain v=? Is there a way you could handle that better?
| }; | ||
|
|
||
| const handleAddVideo = async (title, url) => { | ||
| if (title !== "" && url !== "" && matchYoutubeUrl(url)) { |
There was a problem hiding this comment.
This works, but often times we'll write this like:
if (title === "" || url === "" || !matchYoutubeUrl(url)) {
setShowError(true);
return;
}
try {
...
}Can you talk through why that may be more clear?
| @@ -0,0 +1,15 @@ | |||
| const { Pool } = require('pg'); | |||
|
|
|||
| link = "postgres://vtsohdxm:jNSKq4-96QSdfZax4qdrprDhVwu-rcrF@surus.db.elephantsql.com/vtsohdxm" | |||
There was a problem hiding this comment.
You probably don't want to check this in, containing a password - can you use the dotenv library and a .env file to set this up instead?
| }); | ||
|
|
||
|
|
||
| app.delete("/:id", (req, res) => { |
There was a problem hiding this comment.
Can you update this to work on the database?
|
|
||
| const result = schema.validate(req.body); | ||
| console.log(result) | ||
| const foundVideo = videos.find((booking) => { |
There was a problem hiding this comment.
booking seems like a strange name for this parameter :)
| } | ||
| }); | ||
|
|
||
| app.put("/:id", (req, res) => { |
There was a problem hiding this comment.
Can you update this to work on the database?
4 participants
No description provided.