- implemented CRUD #49
Conversation
There was a problem hiding this comment.
Great job on implementing the core functionalities for managing movies! 🎉 While there are a few minor improvements suggested, such as validating the country field with ISO 3166-1 alpha-3 codes and handling exceptions more specifically, these do not significantly impact the overall functionality. Keep up the good work and consider these suggestions for future enhancements! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| per_page: Annotated[int, Query(ge=1, le=20)] = 10, | ||
| db: Session = Depends(get_db) | ||
| ): | ||
| movies = db.query(MovieModel).order_by(desc(MovieModel.id)).offset((page - 1) * per_page).limit(per_page) |
There was a problem hiding this comment.
Consider checking if the page exceeds total_pages before querying the database to avoid unnecessary queries when the page is out of range.
| try: | ||
| session.add(instance) | ||
| session.commit() | ||
| except Exception: |
There was a problem hiding this comment.
Catching a general Exception is not recommended. Consider catching specific exceptions like IntegrityError to handle database-related issues more accurately.
| @router.post("/movies/", response_model=MovieDetailResponseSchema, status_code=201) | ||
| def add_film(movie: MovieCreateResponseSchema, db: Session = Depends(get_db)): | ||
| movie_dict = movie.dict() | ||
| movie_dict["country"] = get_or_create(db, CountryModel, code=movie_dict["country"]) |
There was a problem hiding this comment.
Ensure that the country field is linked using the ISO 3166-1 alpha-3 code as specified in the task description. Verify that get_or_create handles this correctly.
| status: str | ||
| budget: float = Field(ge=0) | ||
| revenue: float = Field(ge=0) | ||
| country: str |
There was a problem hiding this comment.
The country field in MovieCreateResponseSchema should be validated to ensure it uses the ISO 3166-1 alpha-3 code format, as specified in the task description. Consider adding a regex pattern or a custom validator to enforce this format.
No description provided.