Implement CRUD #10
Implement CRUD #10
Conversation
There was a problem hiding this comment.
Great job on implementing the CRUD operations and API routes for the movie database! 🎉 Your code meets the task requirements and constraints effectively. While there are some minor issues related to validation, error handling, and response models, they don't significantly impact the functionality. Keep up the good work, and consider the feedback for future improvements! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
|
|
||
|
|
||
| def create_movie(movie_data, db: Session = Depends(get_db)): | ||
| if not 0 <= movie_data.score <= 100: |
There was a problem hiding this comment.
The condition not 0 <= movie_data.score <= 100 should include the boundary values. Consider changing it to not (0 <= movie_data.score <= 100) to ensure scores of 0 and 100 are considered valid.
| status_code=404, | ||
| detail="Movie with the given ID was not found." | ||
| ) | ||
| if movie_data.score and not 0 < movie_data.score < 100: |
There was a problem hiding this comment.
The condition not 0 < movie_data.score < 100 should include the boundary values. Consider changing it to not (0 <= movie_data.score <= 100) to ensure scores of 0 and 100 are considered valid.
| detail=f"A movie with the name '{movie.name}'" | ||
| f" and release date '{movie.date}' already exists." |
There was a problem hiding this comment.
There is a potential issue with the error message here. It uses movie.name and movie.date, which might not reflect the intended values if the movie data is being updated. Consider using movie_data.name and movie_data.date instead.
| if not movie: | ||
| raise HTTPException( | ||
| status_code=404, | ||
| detail="Movie with the given ID was not found." | ||
| ) |
There was a problem hiding this comment.
The check for if not movie: is redundant here because the get_movie function already raises an HTTPException if the movie is not found. You can remove this check to avoid unnecessary code duplication.
| ): | ||
| return get_movies(page, per_page, db) | ||
|
|
||
|
|
There was a problem hiding this comment.
The response model for the POST request should not be MovieCreate as it represents the input data structure. Consider using a model that represents the created movie, such as DetailedMovies or another appropriate schema.
| id: int | ||
| name: str | ||
| date: datetime.date | ||
| score: float |
There was a problem hiding this comment.
The score field in MovieSchema should be consistent with the validation logic in the CRUD operations. Ensure that the score is validated to be between 0 and 100, inclusive.
| revenue: float | ||
| status: str | ||
| budget: float |
There was a problem hiding this comment.
Consider making revenue, status, and budget optional in DetailedMovies if they can be absent or unknown in some cases. This would align with the flexibility provided in MovieUpdate.
No description provided.