-
Notifications
You must be signed in to change notification settings - Fork 58
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.
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
Solution #30
base: main
Are you sure you want to change the base?
Solution #30
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on implementing the core functionalities and meeting the task requirements! 🎉 While there are some minor improvements suggested, such as allowing zero values for budget
and revenue
, and adding checks for existing country codes, these do not impact the overall functionality. Keep up the good work and consider these enhancements for future iterations! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
def create_country_by_code(code, model, db): | ||
country = model(code=code) | ||
db.add(country) | ||
db.commit() | ||
db.refresh(country) | ||
return country |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a check to see if a country with the given code already exists in the database before creating a new one. This can prevent duplicate entries and ensure data integrity.
if movie.date > datetime.datetime.now().date() + datetime.timedelta(days=365): | ||
raise HTTPException( | ||
status_code=400, | ||
detail="Invalid input data.", | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the date
field is validated to not be more than one year in the future, as per the task requirements. This check is correctly implemented here.
movie_date = movie_data.model_dump(exclude_unset=True) | ||
for key, value in movie_date.items(): | ||
if value: | ||
setattr(movie, key, value) | ||
db.commit() | ||
db.refresh(movie) | ||
except IntegrityError: | ||
raise HTTPException(status_code=400, detail="Invalid input data.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the edit_movie
function, ensure that the score
is validated to be between 0 and 100, and that budget
and revenue
are non-negative. This is crucial to meet the task's validation requirements.
budget: float = Field(gt=0) | ||
revenue: float = Field(gt=0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider allowing the budget
and revenue
fields to be zero by changing gt=0
to ge=0
in the MovieCreate
schema. This would accommodate scenarios where a movie might have a budget or revenue of zero.
budget: Optional[float] = Field(gt=0, default=None) | ||
revenue: Optional[float] = Field(gt=0, default=None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider allowing the budget
and revenue
fields to be zero by changing gt=0
to ge=0
in the MovieUpdate
schema. This would accommodate scenarios where a movie might have a budget or revenue of zero.
No description provided.