task done #655
task done #655
Conversation
cinema/serializers.py
Outdated
| model = Order | ||
| fields = ("id", "created_at", "tickets") | ||
|
|
||
| def create(self, validated_data): |
There was a problem hiding this comment.
In create/update/delete functions you don't need to return anything
cinema/models.py
Outdated
| @@ -84,24 +84,34 @@ class Ticket(models.Model): | |||
| row = models.IntegerField() | |||
| seat = models.IntegerField() | |||
|
|
|||
| def clean(self): | |||
| @staticmethod | |||
| def validate_ticket(row, seat, cinema_hall, error): | |||
There was a problem hiding this comment.
Fix annotations in all your functions
cinema/views.py
Outdated
| "genres", | ||
| "actors" | ||
| ) | ||
| return queryset |
cinema/views.py
Outdated
| if movie: | ||
| queryset = queryset.filter(movie_id=movie) | ||
|
|
||
| if self.action in "list": |
| return [int(str_id) for str_id in query_string.split(",")] | ||
|
|
||
| def get_queryset(self): | ||
| queryset = self.queryset |
There was a problem hiding this comment.
call super method instead of accesing this attribute directly
also solve N+1 problem
cinema/views.py
Outdated
| @@ -48,11 +79,69 @@ class MovieSessionViewSet(viewsets.ModelViewSet): | |||
| queryset = MovieSession.objects.all() | |||
| serializer_class = MovieSessionSerializer | |||
|
|
|||
| def get_serializer_class(self): | |||
| def get_queryset(self) -> queryset: | |||
| queryset = self.queryset | |||
cinema/serializers.py
Outdated
| ) | ||
|
|
||
|
|
||
| class MovieSessionDetailSerializer(MovieSessionSerializer): | ||
| movie = MovieListSerializer(many=False, read_only=True) | ||
| cinema_hall = CinemaHallSerializer(many=False, read_only=True) | ||
| taken_places = serializers.SerializerMethodField() |
There was a problem hiding this comment.
Use nested serializer here. HINT: create one more TicketSerializer that will have only row and seat fields.
cinema/views.py
Outdated
| return [int(str_id) for str_id in query_string.split(",")] | ||
|
|
||
| def get_queryset(self): | ||
| queryset = super().get_queryset() |
There was a problem hiding this comment.
no need to use super here - just access it through self.queryset
cinema/views.py
Outdated
| def get_queryset(self): | ||
| queryset = super().get_queryset() | ||
|
|
||
| actors = self.request.query_params.get("actors") |
There was a problem hiding this comment.
you need this query params only on the list action
cinema/views.py
Outdated
| def get_queryset(self) -> queryset: | ||
| queryset = super().get_queryset() | ||
|
|
||
| date = self.request.query_params.get("date") |
There was a problem hiding this comment.
Again - you need these query params only on list action
cinema/views.py
Outdated
| if self.action in "list": | ||
| queryset = ( | ||
| queryset | ||
| .prefetch_related("movie", "cinema_hall", "tickets") |
There was a problem hiding this comment.
movie and cinema_hall needs to be select_ not prefetch_
cinema/views.py
Outdated
| if self.action == "list": | ||
| return MovieSessionListSerializer | ||
|
|
||
| if self.action == "retrieve": | ||
| return MovieSessionDetailSerializer | ||
|
|
||
| return MovieSessionSerializer | ||
|
|
||
|
|
||
| class OrderSetPagination(PageNumberPagination): |
There was a problem hiding this comment.
Better move pagination to a separate file to better structure your code
cinema/views.py
Outdated
|
|
||
|
|
||
| class OrderViewSet(viewsets.ModelViewSet): | ||
| queryset = Order.objects.all() |
There was a problem hiding this comment.
this is never used because you override queryset in method below
cinema/serializers.py
Outdated
| ) | ||
|
|
||
|
|
||
| class TicketRowAndSeatSerializer(serializers.ModelSerializer): | ||
| row = serializers.IntegerField() |
There was a problem hiding this comment.
better like this instead:
class Meta:
model = Ticket
fields = ("row", "seat")
cinema/serializers.py
Outdated
| class MovieSessionDetailSerializer(MovieSessionSerializer): | ||
| movie = MovieListSerializer(many=False, read_only=True) | ||
| cinema_hall = CinemaHallSerializer(many=False, read_only=True) | ||
| taken_places = serializers.SerializerMethodField() |
There was a problem hiding this comment.
so here you need to pass your TicketRowAndSeatSerializer and then you will not need your get_taken_seats method
cinema/views.py
Outdated
| if movie: | ||
| queryset = queryset.filter(movie_id=movie) | ||
|
|
||
| if self.action in "list": |
There was a problem hiding this comment.
why do you check if action equals list twice? can you optimize here?
No description provided.