-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Implementation all functions #1096
base: master
Are you sure you want to change the base?
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 task requirements! 🎉 The code is well-structured and meets the core functionality needed. While there are a few areas where handling non-existent records could be improved to prevent potential exceptions, these are minor and don't affect 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! ✨
('description', models.TextField()), | ||
('actors', models.ManyToManyField(to='db.actor')), | ||
('genres', models.ManyToManyField(to='db.genre')), |
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 actor
and genre
models exist in the db
app, as they are referenced in the ManyToManyField
relationships. If they do not exist, this will cause errors during migration.
def get_movie_by_id(movie_id: int) -> Movie: | ||
return Movie.objects.get(id=movie_id) |
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 handling the case where a movie with the given movie_id
does not exist. You can use Movie.objects.filter(id=movie_id).first()
to return None
if no movie is found, or handle the DoesNotExist
exception if using get
.
|
||
def get_movie_session_by_id(movie_session_id: int) -> MovieSession: | ||
return MovieSession.objects.get(id=movie_session_id) |
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 handling the case where a movie session with the given movie_session_id
does not exist. You can use MovieSession.objects.filter(id=movie_session_id).first()
to return None
if no session is found, or handle the DoesNotExist
exception if using get
.
services/movie_session.py
Outdated
|
||
def update_movie_session(session_id, | ||
show_time: datetime = None, | ||
movie_id: int = None, | ||
cinema_hall_id: int = None) -> None: | ||
session = MovieSession.objects.get(id=session_id) | ||
if show_time: | ||
session.show_time = show_time | ||
|
||
if movie_id: | ||
session.movie_id = movie_id | ||
|
||
if cinema_hall_id: | ||
session.cinema_hall_id = cinema_hall_id | ||
|
||
session.save() |
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 update_movie_session
, consider handling the case where a session with the given session_id
does not exist. This will prevent exceptions if the session ID is invalid.
services/movie_session.py
Outdated
|
||
def delete_movie_session_by_id(session_id: int): | ||
return MovieSession.objects.get(id=session_id).delete() |
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 delete_movie_session_by_id
, consider handling the case where a session with the given session_id
does not exist. This will prevent exceptions if the session ID is invalid.
No description provided.