Solution #718
Solution #718
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this pull request! 😊 While your code is well-structured and meets most of the requirements, there are a few critical issues that need to be addressed before we can approve it:
-
Validation in
OrderSerializer: Thecreatemethod inOrderSerializerdoes not validate theticketsdata before creatingTicketinstances. This could lead to invalid data being saved to the database. Please useTicketSerializerto validate each ticket entry to ensure data integrity. -
Queryset Modification: In both
MovieViewSetandMovieSessionViewSet, you are modifyingself.querysetdirectly in theget_querysetmethod. This can lead to unexpected behavior if the method is called multiple times. Consider using a local variable to store the filtered queryset and return it at the end of the method.
These changes are crucial for maintaining the functionality and reliability of your application. Once these issues are resolved, your code will be in great shape! Keep up the great work, and don't hesitate to reach out if you have any questions or need further assistance. 🚀
✨ 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.
|
|
||
|
|
||
| class OrderSerializer(serializers.ModelSerializer): | ||
| tickets = TicketSerializer(many=True, read_only=False, allow_empty=False) |
There was a problem hiding this comment.
The tickets field in OrderSerializer is set to read_only=False, which means you expect to receive data for tickets when creating an order. However, the create method does not validate this data using the TicketSerializer. Consider using TicketSerializer to validate each ticket entry before creating Ticket instances in the create method.
| def create(self, validated_data): | ||
| with transaction.atomic(): | ||
| tickets_data = validated_data.pop("tickets") | ||
| order = Order.objects.create(**validated_data) | ||
| for ticket_data in tickets_data: | ||
| Ticket.objects.create(order=order, **ticket_data) | ||
|
|
||
| return order |
There was a problem hiding this comment.
In the create method of OrderSerializer, you should validate tickets_data using TicketSerializer before creating Ticket instances. This ensures that only valid data is saved to the database. You can iterate over tickets_data, use TicketSerializer(data=ticket_data) for each item, call is_valid(), and then save() if valid.
There was a problem hiding this comment.
Great job on your progress! 🎉 Your implementation is well-structured, and you've made significant improvements. Here are a few key points to focus on:
-
Validation in
OrderSerializer: It's crucial to validate theticketsdata usingTicketSerializerbefore creatingTicketinstances. This ensures data integrity and prevents invalid data from being saved to the database. Please iterate overtickets_dataand useTicketSerializer(data=ticket_data)for validation. -
Queryset Modification: You've successfully addressed the issue of modifying
self.querysetdirectly in theget_querysetmethods of bothMovieViewSetandMovieSessionViewSet. Using a local variable to store the filtered queryset is a great approach.
Remember, addressing these issues will enhance the robustness and reliability of your code. Keep up the good work, and continue learning from these experiences. Your dedication to improving your skills is commendable! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| def create(self, validated_data): | ||
| with transaction.atomic(): | ||
| tickets_data = validated_data.pop("tickets") | ||
| order = Order.objects.create(**validated_data) | ||
| for ticket_data in tickets_data: | ||
| Ticket.objects.create(order=order, **ticket_data) | ||
|
|
||
| return order |
There was a problem hiding this comment.
The create method in OrderSerializer does not validate the tickets data before creating Ticket instances. This could lead to invalid data being saved to the database. Please use TicketSerializer to validate each ticket entry to ensure data integrity. You can iterate over tickets_data and use TicketSerializer(data=ticket_data) to validate each ticket.
| queryset = self.queryset | ||
|
|
||
| genres = self.request.query_params.get("genres") | ||
| actors = self.request.query_params.get("actors") | ||
| title = self.request.query_params.get("title") | ||
|
|
||
| if genres: | ||
| genres = self._params_to_ints(genres) | ||
| queryset = queryset.filter(genres__id__in=genres) | ||
|
|
||
| if actors: | ||
| actors = self._params_to_ints(actors) | ||
| queryset = queryset.filter(actors__id__in=actors) | ||
|
|
||
| if title: | ||
| queryset = queryset.filter(title__icontains=title) | ||
|
|
||
| return queryset.distinct() |
There was a problem hiding this comment.
In the get_queryset method of MovieViewSet, you are modifying self.queryset directly. This can lead to unexpected behavior if the method is called multiple times. Consider using a local variable to store the filtered queryset and return it at the end of the method.
| queryset = self.queryset | ||
|
|
||
| date = self.request.query_params.get("date") | ||
| movie = self.request.query_params.get("movie") | ||
|
|
||
| if date: | ||
| queryset = queryset.filter(show_time__date=date) | ||
|
|
||
| if movie: | ||
| movie_ids = [int(str_id) for str_id in movie.split(",")] | ||
| queryset = queryset.filter(movie__id__in=movie_ids) | ||
|
|
||
| if self.action == "list": | ||
| queryset = queryset.select_related( | ||
| "cinema_hall", | ||
| "movie", | ||
| ).annotate( | ||
| tickets_available=F("cinema_hall__rows") | ||
| * F("cinema_hall__seats_in_row") | ||
| - Count("tickets") | ||
| ) | ||
|
|
||
| return queryset.distinct() |
There was a problem hiding this comment.
In the get_queryset method of MovieSessionViewSet, you are modifying self.queryset directly. This can lead to unexpected behavior if the method is called multiple times. Consider using a local variable to store the filtered queryset and return it at the end of the method.
No description provided.