Skip to content
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

solution #676

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ venv/
.pytest_cache/
**__pycache__/
*.pyc
app/db.sqlite3
db.sqlite3
2 changes: 1 addition & 1 deletion cinema/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class Actor(models.Model):
last_name = models.CharField(max_length=255)

def __str__(self):
return self.first_name + " " + self.last_name
return f"{self.first_name} {self.last_name}"

@property
def full_name(self):
Expand Down
68 changes: 66 additions & 2 deletions cinema/serializers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
from django.db import transaction
from rest_framework import serializers

from cinema.models import Genre, Actor, CinemaHall, Movie, MovieSession
from cinema.models import (
Genre,
Actor,
CinemaHall,
Movie,
MovieSession,
Ticket,
Order
)


class GenreSerializer(serializers.ModelSerializer):
Expand Down Expand Up @@ -59,6 +68,7 @@ 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
Expand All @@ -68,13 +78,67 @@ class Meta:
"movie_title",
"cinema_hall_name",
"cinema_hall_capacity",
"tickets_available"
)


class TicketSerializer(serializers.ModelSerializer):
class Meta:
model = Ticket
fields = ("id", "row", "seat", "movie_session")


class TicketListSerializer(TicketSerializer):
movie_session = MovieSessionListSerializer(read_only=True)

class Meta:
model = Ticket
fields = ("id", "row", "seat", "movie_session")


class TicketTakenPlacesSerializer(serializers.ModelSerializer):
class Meta:
model = Ticket
fields = ("row", "seat")


class MovieSessionDetailSerializer(MovieSessionSerializer):
movie = MovieListSerializer(many=False, read_only=True)
cinema_hall = CinemaHallSerializer(many=False, read_only=True)
taken_places = TicketTakenPlacesSerializer(
many=True,
read_only=True,
source="tickets"
)

class Meta:
model = MovieSession
fields = ("id", "show_time", "movie", "cinema_hall")
fields = ("id", "show_time", "movie", "cinema_hall", "taken_places")


class OrderSerializer(serializers.ModelSerializer):
tickets = TicketSerializer(
many=True,
read_only=False,
allow_empty=False,
)

class Meta:
model = Order
fields = (
"id",
"tickets",
"created_at",
)

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


class OrderListSerializer(OrderSerializer):
tickets = TicketListSerializer(many=True, read_only=True)
2 changes: 2 additions & 0 deletions cinema/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
CinemaHallViewSet,
MovieViewSet,
MovieSessionViewSet,
OrderViewSet,
)

router = routers.DefaultRouter()
Expand All @@ -15,6 +16,7 @@
router.register("cinema_halls", CinemaHallViewSet)
router.register("movies", MovieViewSet)
router.register("movie_sessions", MovieSessionViewSet)
router.register("orders", OrderViewSet)

urlpatterns = [path("", include(router.urls))]

Expand Down
94 changes: 90 additions & 4 deletions cinema/views.py
Original file line number Diff line number Diff line change
@@ -1,6 +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,
Expand All @@ -11,7 +13,7 @@
MovieSessionListSerializer,
MovieDetailSerializer,
MovieSessionDetailSerializer,
MovieListSerializer,
MovieListSerializer, OrderSerializer, OrderListSerializer,
)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Style: Group imports using () if needed. The import statement for the serializers is not grouped correctly. The trailing comma should be removed from the last import inside the parentheses.



Expand Down Expand Up @@ -43,16 +45,100 @@ def get_serializer_class(self):

return MovieSerializer

@staticmethod
def _params_to_int(query_string):
return [int(str_id) for str_id in query_string.split(",")]

def get_queryset(self):
queryset = self.queryset
actors = self.request.query_params.get("actors")
genres = self.request.query_params.get("genres")
title = self.request.query_params.get("title")

if actors:
actors = self._params_to_int(actors)
queryset = queryset.filter(actors__id__in=actors)

elif genres:
genres = self._params_to_int(genres)
queryset = queryset.filter(genres__id__in=genres)
elif title:
queryset = queryset.filter(title__icontains=title)
return queryset


class MovieSessionViewSet(viewsets.ModelViewSet):
queryset = MovieSession.objects.all()
serializer_class = MovieSessionSerializer

@staticmethod
def _params_to_int(query_string):
return [int(str_id) for str_id in query_string.split(",")]

def get_serializer_class(self):
if self.action == "list":
return MovieSessionListSerializer

if self.action == "retrieve":
elif self.action == "retrieve":
return MovieSessionDetailSerializer

return MovieSessionSerializer

def get_queryset(self):
queryset = self.queryset
if self.action in "list":
queryset = queryset.select_related().annotate(
tickets_available=(
F("cinema_hall__rows")
* F("cinema_hall__seats_in_row")
- Count("tickets")
)
)

date = self.request.query_params.get("date")
movie = self.request.query_params.get("movie")
if date and movie:
movie = self._params_to_int(movie)
queryset = queryset.filter(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Efficiency: Don't split the date, it's already in the format needed. The date variable is used in the filter without being split into year, month, and day.

show_time__date=date,
movie__id__in=movie
)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Efficiency: Don't split the date, it's already in the format needed. The date variable is used in the filter without being split into year, month, and day.

elif date:
queryset = queryset.filter(
show_time__date=date
)
Comment on lines +98 to +108

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Efficiency issue: It seems you are splitting the date to filter the queryset (lines 101 and 107). According to the checklist, this is not an efficient way to filter by date. Try filtering the date directly without splitting it. Refer to the checklist for a good example.

elif movie:
movie = self._params_to_int(movie)
queryset = queryset.filter(movie__id__in=movie)
return queryset
Comment on lines +96 to +112

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Quality issue: The if-elif-elif condition in lines 99-112 is not optimal. If the 'date' and 'movie' parameters are both present in the request, then the filter will be applied correctly. But if only 'movie' is present, it will be ignored because of the 'elif' condition. Consider changing the 'elif' conditions to 'if' so that each parameter is checked and applied independently.



class OrderPagination(PageNumberPagination):
page_size = 5
page_size_query_param = "page_size"
max_page_size = 10


class OrderViewSet(viewsets.ModelViewSet):
serializer_class = OrderSerializer
queryset = Order.objects.all()
pagination_class = OrderPagination

def get_queryset(self):
queryset = self.queryset.filter(user=self.request.user)
if self.action == "list":
queryset = queryset.prefetch_related(
"tickets__movie_session__cinema_hall",
"tickets__movie_session__movie",
)
return queryset

def perform_create(self, serializer):
serializer.save(user=self.request.user)

def get_serializer_class(self):
serializer = self.serializer_class

if self.action == "list":
serializer = OrderListSerializer

return serializer
2 changes: 1 addition & 1 deletion cinema_service/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@

USE_I18N = True

USE_TZ = False
USE_TZ = True


# Static files (CSS, JavaScript, Images)
Expand Down
Loading