-
Notifications
You must be signed in to change notification settings - Fork 636
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
task done #655
base: master
Are you sure you want to change the base?
task done #655
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,16 @@ | ||
from django.db import transaction | ||
from rest_framework import serializers | ||
from rest_framework.exceptions import ValidationError | ||
|
||
from cinema.models import Genre, Actor, CinemaHall, Movie, MovieSession | ||
from cinema.models import ( | ||
Genre, | ||
Actor, | ||
CinemaHall, | ||
Movie, | ||
MovieSession, | ||
Order, | ||
Ticket | ||
) | ||
|
||
|
||
class GenreSerializer(serializers.ModelSerializer): | ||
|
@@ -59,6 +69,9 @@ class MovieSessionListSerializer(MovieSessionSerializer): | |
cinema_hall_capacity = serializers.IntegerField( | ||
source="cinema_hall.capacity", read_only=True | ||
) | ||
tickets_available = serializers.IntegerField( | ||
read_only=True | ||
) | ||
|
||
class Meta: | ||
model = MovieSession | ||
|
@@ -68,13 +81,70 @@ class Meta: | |
"movie_title", | ||
"cinema_hall_name", | ||
"cinema_hall_capacity", | ||
"tickets_available" | ||
) | ||
|
||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use nested serializer here. HINT: create one more TicketSerializer that will have only row and seat fields. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so here you need to pass your TicketRowAndSeatSerializer and then you will not need your get_taken_seats method |
||
|
||
class Meta: | ||
model = MovieSession | ||
fields = ("id", "show_time", "movie", "cinema_hall") | ||
fields = ("id", "show_time", "movie", "cinema_hall", "taken_places") | ||
|
||
def get_taken_places(self, obj): | ||
tickets = Ticket.objects.filter(movie_session=obj) | ||
return [{"row": ticket.row, "seat": ticket.seat} for ticket in tickets] | ||
|
||
|
||
class TicketSerializer(serializers.ModelSerializer): | ||
movie_session = MovieSessionListSerializer() | ||
|
||
class Meta: | ||
model = Ticket | ||
fields = ("id", "row", "seat", "movie_session") | ||
|
||
def validate(self, attrs): | ||
Ticket.validate_ticket( | ||
attrs["row"], | ||
attrs["seat"], | ||
attrs["movie_session"].cinema_hall, | ||
ValidationError | ||
) | ||
return attrs | ||
|
||
|
||
class TicketCreateSerializer(TicketSerializer): | ||
movie_session = serializers.PrimaryKeyRelatedField( | ||
queryset=MovieSession.objects.all() | ||
) | ||
|
||
|
||
class OrderSerializer(serializers.ModelSerializer): | ||
tickets = TicketCreateSerializer( | ||
many=True, | ||
read_only=False, | ||
allow_empty=False | ||
) | ||
|
||
class Meta: | ||
model = Order | ||
fields = ("id", "created_at", "tickets") | ||
|
||
def create(self, validated_data): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In create/update/delete functions you don't need to return anything |
||
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 | ||
|
||
|
||
class OrderRetrieveSerializer(OrderSerializer): | ||
tickets = TicketSerializer(many=True, read_only=True) | ||
|
||
class Meta: | ||
model = Order | ||
fields = ("id", "created_at", "tickets") |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
from django.db.models import Count, F | ||
from rest_framework import viewsets | ||
from rest_framework.pagination import PageNumberPagination | ||
|
||
from cinema.models import Genre, Actor, CinemaHall, Movie, MovieSession | ||
|
||
from cinema.models import Genre, Actor, CinemaHall, Movie, MovieSession, Order | ||
from cinema.serializers import ( | ||
GenreSerializer, | ||
ActorSerializer, | ||
|
@@ -12,6 +13,8 @@ | |
MovieDetailSerializer, | ||
MovieSessionDetailSerializer, | ||
MovieListSerializer, | ||
OrderSerializer, | ||
OrderRetrieveSerializer, | ||
) | ||
|
||
|
||
|
@@ -34,6 +37,33 @@ class MovieViewSet(viewsets.ModelViewSet): | |
queryset = Movie.objects.all() | ||
serializer_class = MovieSerializer | ||
|
||
@staticmethod | ||
def _params_to_ints(query_string): | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. call super method instead of accesing this attribute directly |
||
|
||
actors = self.request.query_params.get("actors") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you need this query params only on the list action |
||
genres = self.request.query_params.get("genres") | ||
titles = self.request.query_params.get("title") | ||
|
||
if actors: | ||
actors = self._params_to_ints(actors) | ||
queryset = queryset.filter(actors__id__in=actors) | ||
elif genres: | ||
genres = self._params_to_ints(genres) | ||
queryset = queryset.filter(genres__id__in=genres) | ||
elif titles: | ||
queryset = queryset.filter(title__icontains=titles) | ||
|
||
if self.action == "list": | ||
queryset = queryset.prefetch_related( | ||
"genres", | ||
"actors" | ||
) | ||
return queryset | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To use distinct will be good here |
||
|
||
def get_serializer_class(self): | ||
if self.action == "list": | ||
return MovieListSerializer | ||
|
@@ -48,6 +78,29 @@ class MovieSessionViewSet(viewsets.ModelViewSet): | |
queryset = MovieSession.objects.all() | ||
serializer_class = MovieSessionSerializer | ||
|
||
def get_queryset(self): | ||
queryset = self.queryset | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here the same |
||
|
||
date = self.request.query_params.get("date") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again - you need these query params only on list action |
||
movie = self.request.query_params.get("movie") | ||
|
||
if date: | ||
queryset = queryset.filter(show_time__date=date) | ||
if movie: | ||
queryset = queryset.filter(movie_id=movie) | ||
|
||
if self.action in "list": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And if action is retrieve? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do you check if action equals list twice? can you optimize here? |
||
queryset = ( | ||
queryset | ||
.prefetch_related("movie", "cinema_hall", "tickets") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. movie and cinema_hall needs to be select_ not prefetch_ |
||
.annotate( | ||
capacity=(F("cinema_hall__rows") | ||
* F("cinema_hall__seats_in_row")), | ||
tickets_available=F("capacity") - Count("tickets")) | ||
) | ||
|
||
return queryset | ||
|
||
def get_serializer_class(self): | ||
if self.action == "list": | ||
return MovieSessionListSerializer | ||
|
@@ -56,3 +109,32 @@ def get_serializer_class(self): | |
return MovieSessionDetailSerializer | ||
|
||
return MovieSessionSerializer | ||
|
||
|
||
class OrderSetPagination(PageNumberPagination): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better move pagination to a separate file to better structure your code |
||
page_size = 1 | ||
page_query_param = "page_size" | ||
max_page_size = 20 | ||
|
||
|
||
class OrderViewSet(viewsets.ModelViewSet): | ||
queryset = Order.objects.all() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is never used because you override queryset in method below |
||
pagination_class = OrderSetPagination | ||
|
||
def get_queryset(self): | ||
queryset = Order.objects.filter(user=self.request.user) | ||
|
||
if self.action == "list": | ||
queryset = queryset.prefetch_related( | ||
"tickets__movie_session__movie", | ||
"tickets__movie_session__cinema_hall" | ||
) | ||
return queryset | ||
|
||
def get_serializer_class(self): | ||
if self.action in ("list", "retrieve"): | ||
return OrderRetrieveSerializer | ||
return OrderSerializer | ||
|
||
def perform_create(self, serializer): | ||
serializer.save(user=self.request.user) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -124,7 +124,7 @@ | |
|
||
USE_I18N = True | ||
|
||
USE_TZ = False | ||
USE_TZ = True | ||
|
||
|
||
# Static files (CSS, JavaScript, Images) | ||
|
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.
Fix annotations in all your functions