solution #1093
solution #1093
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this project! However, there are a few critical issues that need to be addressed before we can approve the pull request. These issues impact the functionality and core requirements of the code:
-
Typographical Errors:
- In
models.py, theMoviemodel has a typo in the field namegerne, which should begenre. - In
movie.py, thecreate_moviefunction has a typo:gernesshould begenres. - In
movie_session.py, there are typos inget_movies_sessions(objectashould beobjects) andget_movie_session_by_id(movie_sessin_idshould bemovie_session_id).
- In
-
Incorrect Method Usage:
- The
get_movie_by_idfunction should use.get()instead of.filter()to retrieve a singleMovieinstance.
- The
-
Persistence Issues:
- The
create_cinema_hallfunction does not save the newCinemaHallinstance to the database, which is necessary for persistence.
- The
-
Filter Application:
- The
filtermethod is not being applied correctly in bothget_movies_sessionsandget_movies. You need to assign the result of thefiltermethod back to the variable to actually apply the filters.
- The
Please address these issues to ensure the code functions as expected. Once these changes are made, we can proceed with the approval. Keep up the great work, and don't hesitate to reach out if you need any help! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
db/models.py
Outdated
| title = models.CharField(max_length=255, unique=True) | ||
| description = models.TextField() | ||
| actors = models.ManyToManyField(Actor) | ||
| gerne = models.ManyToManyField(Genre) |
There was a problem hiding this comment.
There is a typo here: gerne should be genre. This will cause an error when trying to access this field.
services/cinema_hall.py
Outdated
| def create_cinema_hall(hall_name: str, | ||
| hall_rows: int, | ||
| hall_seats_in_row: int) -> CinemaHall: | ||
| return CinemaHall(hall_name, | ||
| hall_rows, | ||
| hall_seats_in_row) |
There was a problem hiding this comment.
The create_cinema_hall function creates a CinemaHall instance but does not save it to the database. You should call the save() method on the CinemaHall instance to persist it to the database.
services/movie.py
Outdated
| movies.filter(genres__in=genres_ids) | ||
| if actors_ids: | ||
| movies.filter(actors__in=actors_ids) |
There was a problem hiding this comment.
The filter method is not being applied correctly. You need to assign the result of the filter method back to the movies variable, like movies = movies.filter(...), to actually apply the filters.
services/movie.py
Outdated
|
|
||
|
|
||
| def get_movie_by_id(movie_id) -> Movie: | ||
| return Movie.objects.filter(id=movie_id) |
There was a problem hiding this comment.
The get_movie_by_id function should use get() instead of filter() to retrieve a single instance. Using filter() returns a QuerySet, not a single Movie instance.
services/movie.py
Outdated
| new_movie = Movie.objects.create(title=movie_title, | ||
| description=movie_description) | ||
| if genres_ids: | ||
| new_movie.gernes.set(genres_ids) |
There was a problem hiding this comment.
There is a typo here: gernes should be genres. This will cause an error when trying to set the genres for the movie.
services/movie_session.py
Outdated
|
|
||
|
|
||
| def get_movies_sessions(session_date: datetime.date = None) -> QuerySet: | ||
| sessions_list = MovieSession.objecta.all() |
There was a problem hiding this comment.
There is a typo here: objecta should be objects. This will cause an error when trying to retrieve all movie sessions.
services/movie_session.py
Outdated
| def get_movies_sessions(session_date: datetime.date = None) -> QuerySet: | ||
| sessions_list = MovieSession.objecta.all() | ||
| if session_date: | ||
| sessions_list.filter(show_time__date=session_date) |
There was a problem hiding this comment.
The filter method is not being applied correctly. You need to assign the result of the filter method back to the sessions_list variable, like sessions_list = sessions_list.filter(...), to actually apply the filter.
services/movie_session.py
Outdated
|
|
||
|
|
||
| def get_movie_session_by_id(movie_session_id: int) -> MovieSession: | ||
| return MovieSession.objects.get(id=movie_sessin_id) |
There was a problem hiding this comment.
There is a typo here: movie_sessin_id should be movie_session_id. This will cause an error when trying to retrieve a movie session by ID.
There was a problem hiding this comment.
Great effort on addressing some of the issues from the previous review! 🎉 While the Pull Request is approved to help you move forward with your learning, there are still some critical issues that need attention to ensure the code functions as expected:
-
Typographical Errors:
- In
services/movie.py, thecreate_moviefunction still contains a typo:gernesshould be corrected togenres.
- In
-
Incorrect Method Usage:
- The
get_movie_by_idfunction inservices/movie.pyshould use.get()instead of.filter()to retrieve a singleMovieinstance. This change will ensure that an exception is raised if the movie is not found, which is typically the desired behavior.
- The
-
Persistence Issues:
- The
create_cinema_hallfunction inservices/cinema_hall.pydoes not currently save the newCinemaHallinstance to the database. Ensure you call thesave()method on the instance or useCinemaHall.objects.create()for persistence.
- The
-
Filter Application:
- In
services/movie_session.py, thefiltermethod is not applied correctly inget_movies_sessions. Assign the result of thefiltermethod back tosessions_listto apply the filter. - Similarly, in
services/movie.py, ensure thefiltermethod is applied correctly inget_moviesby assigning the result back tomovies.
- In
These issues are crucial for the functionality of your code, so please address them in future iterations. Keep up the good work, and continue learning from these experiences to improve your coding skills! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
services/cinema_hall.py
Outdated
| def create_cinema_hall(hall_name: str, | ||
| hall_rows: int, | ||
| hall_seats_in_row: int) -> CinemaHall: | ||
| return CinemaHall(hall_name, | ||
| hall_rows, | ||
| hall_seats_in_row) |
There was a problem hiding this comment.
The create_cinema_hall function should save the new CinemaHall instance to the database. You can do this by calling the save() method on the instance before returning it. This will ensure the cinema hall is persisted in the database.
services/movie.py
Outdated
| movies.filter(genres__in=genres_ids) | ||
| if actors_ids: | ||
| movies.filter(actors__in=actors_ids) |
There was a problem hiding this comment.
The filter method is not being applied correctly in the get_movies function. You need to assign the result of the filter method back to the movies variable to actually apply the filters. For example: movies = movies.filter(genres__in=genres_ids).
services/movie.py
Outdated
|
|
||
|
|
||
| def get_movie_by_id(movie_id: int) -> Movie: | ||
| return Movie.objects.filter(id=movie_id) |
There was a problem hiding this comment.
The get_movie_by_id function should use .get() instead of .filter() to retrieve a single Movie instance. This will raise an exception if the movie is not found, which is typically the desired behavior when fetching a single object by ID.
services/movie.py
Outdated
| new_movie = Movie.objects.create(title=movie_title, | ||
| description=movie_description) | ||
| if genres_ids: | ||
| new_movie.gernes.set(genres_ids) |
There was a problem hiding this comment.
There is a typo in the create_movie function. The method new_movie.gernes.set(genres_ids) should be new_movie.genres.set(genres_ids) to match the corrected field name in the Movie model.
services/movie_session.py
Outdated
| def get_movies_sessions(session_date: datetime.date = None) -> QuerySet: | ||
| sessions_list = MovieSession.objects.all() | ||
| if session_date: | ||
| sessions_list.filter(show_time__date=session_date) |
There was a problem hiding this comment.
The filter method is not being applied correctly in the get_movies_sessions function. You need to assign the result of the filter method back to the sessions_list variable to actually apply the filter. For example: sessions_list = sessions_list.filter(show_time__date=session_date).
No description provided.