"Solution" #656
"Solution" #656
Conversation
| 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 create/update/delete function you don't need to return anything
cinema/views.py
Outdated
| @@ -11,7 +14,7 @@ | |||
| MovieSessionListSerializer, | |||
| MovieDetailSerializer, | |||
| MovieSessionDetailSerializer, | |||
| MovieListSerializer, | |||
| MovieListSerializer, OrderSerializer, | |||
cinema/views.py
Outdated
| if genres: | ||
| genres_ids = [int(str_id) for str_id in genres.split(",")] | ||
| queryset = queryset.filter(genres__id__in=genres_ids) | ||
|
|
||
| actors = self.request.query_params.get("actors") | ||
| if actors: | ||
| actors_ids = [int(str_id) for str_id in actors.split(",")] | ||
| queryset = queryset.filter(actors__id__in=actors_ids) | ||
|
|
||
| title = self.request.query_params.get("title") | ||
| if title: | ||
| queryset = queryset.filter(title__icontains=title) |
There was a problem hiding this comment.
Better to check all of these things only when self.action == "list"
cinema/views.py
Outdated
|
|
||
| actors = self.request.query_params.get("actors") | ||
| if actors: | ||
| actors_ids = [int(str_id) for str_id in actors.split(",")] |
There was a problem hiding this comment.
You have a duplicates, create a separate function for this
cinema/views.py
Outdated
| @@ -48,6 +73,32 @@ class MovieSessionViewSet(viewsets.ModelViewSet): | |||
| queryset = MovieSession.objects.all() | |||
| serializer_class = MovieSessionSerializer | |||
|
|
|||
| def get_queryset(self): | |||
| queryset = MovieSession.objects.all() | |||
There was a problem hiding this comment.
First will be good to solve N+1 problem
So you can wrap your code in if self.actions == "list" also
And don't forget about retrieve
There was a problem hiding this comment.
You should fix n+1 with movie and cinema_hall
cinema/serializers.py
Outdated
|
|
||
| class Meta: | ||
| model = Ticket | ||
| fields = ("id", "row", "seat", "movie_session", "order") |
There was a problem hiding this comment.
you don't need to display order on a ticket - in this task we actually display a ticket inside of the Order
cinema/serializers.py
Outdated
| class MovieSessionDetailSerializer(MovieSessionSerializer): | ||
| movie = MovieListSerializer(many=False, read_only=True) | ||
| cinema_hall = CinemaHallSerializer(many=False, read_only=True) | ||
| taken_places = TakenPlacesSerializer( | ||
| source="tickets", | ||
| many=True, read_only=True |
cinema/urls.py
Outdated
| @@ -6,7 +6,7 @@ | |||
| ActorViewSet, | |||
| CinemaHallViewSet, | |||
| MovieViewSet, | |||
| MovieSessionViewSet, | |||
| MovieSessionViewSet, OrderViewSet, | |||
| model = Ticket | ||
| fields = ("row", "seat") | ||
|
|
||
|
|
||
| class MovieSessionDetailSerializer(MovieSessionSerializer): |
There was a problem hiding this comment.
What bout MovieSessionSerializer that will be used to create movie session - it must have a bit different signature
| fields = ("id", "show_time", "movie", "cinema_hall", "taken_places") | ||
|
|
||
|
|
||
| class OrderSerializer(serializers.ModelSerializer): |
There was a problem hiding this comment.
What about OrderSerializer that will be used with list action? It will have a different signature
| if self.action == "retrieve": | ||
| return MovieDetailSerializer | ||
|
|
||
| return MovieSerializer | ||
|
|
||
|
|
||
| class MovieSessionViewSet(viewsets.ModelViewSet): | ||
| queryset = MovieSession.objects.all() |
There was a problem hiding this comment.
you don't need this if you define get_queryset method
cinema/views.py
Outdated
| @@ -48,6 +73,32 @@ class MovieSessionViewSet(viewsets.ModelViewSet): | |||
| queryset = MovieSession.objects.all() | |||
| serializer_class = MovieSessionSerializer | |||
|
|
|||
| def get_queryset(self): | |||
| queryset = MovieSession.objects.all() | |||
There was a problem hiding this comment.
You should fix n+1 with movie and cinema_hall
| total_capacity=F( | ||
| "cinema_hall__rows") * F("cinema_hall__seats_in_row"), | ||
| tickets_sold=Count("tickets"), | ||
| tickets_available=F("total_capacity") - F("tickets_sold") |
There was a problem hiding this comment.
tickets available and taken seats must be displayed on different actions and different serializers
|
|
||
|
|
||
| class OrderViewSet(viewsets.ModelViewSet): | ||
| queryset = Order.objects.all() |
There was a problem hiding this comment.
N+1 problem is present here since you access a lot of nested fields
|
|
||
| class OrderViewSet(viewsets.ModelViewSet): | ||
| queryset = Order.objects.all() | ||
| serializer_class = OrderSerializer |
There was a problem hiding this comment.
you will need to add a different serializer for list action and override get_serializer_class method here
No description provided.