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 #742

Open
wants to merge 2 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
46 changes: 31 additions & 15 deletions cinema/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,23 +84,39 @@ class Ticket(models.Model):
row = models.IntegerField()
seat = models.IntegerField()

@staticmethod
def validate_ticket(
row: int,
seat: int,
movie_session: MovieSession,
error_to_raise

Choose a reason for hiding this comment

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

The parameter error_to_raise is unconventional for a validation method. Typically, validation methods directly raise ValidationError without passing an error class as a parameter. Consider removing this parameter and directly raising ValidationError.

Choose a reason for hiding this comment

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

The error_to_raise parameter is expected to be a callable exception class. Ensure that it is always passed a valid exception class that can be instantiated with a dictionary of errors. Consider adding a check to ensure error_to_raise is callable before using it.

):
cinema_hall = movie_session.cinema_hall
errors = {}

constraints = [
(row, "row", cinema_hall.rows),
(seat, "seat", cinema_hall.seats_in_row)
]

for value, attr_name, limit in constraints:
if not (1 <= value <= limit):
errors[attr_name] = (f"{attr_name.capitalize()} "
f"must be in range 1 to {limit}")

if errors:
raise error_to_raise(errors)

def clean(self):
for ticket_attr_value, ticket_attr_name, cinema_hall_attr_name in [
(self.row, "row", "rows"),
(self.seat, "seat", "seats_in_row"),
]:
count_attrs = getattr(
self.movie_session.cinema_hall, cinema_hall_attr_name
try:
Ticket.validate_ticket(
row=self.row,
seat=self.seat,
movie_session=self.movie_session,
error_to_raise=ValidationError
)
if not (1 <= ticket_attr_value <= count_attrs):
raise ValidationError(
{
ticket_attr_name: f"{ticket_attr_name} "
f"number must be in available range: "
f"(1, {cinema_hall_attr_name}): "
f"(1, {count_attrs})"
}
)
except ValidationError as e:
raise ValidationError(e.message_dict)
Comment on lines +118 to +119

Choose a reason for hiding this comment

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

In the clean method, catching and re-raising ValidationError is unnecessary. The validate_ticket method should directly raise ValidationError, and the clean method should allow it to propagate naturally.


def save(
self,
Expand Down
95 changes: 93 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.SerializerMethodField()

class Meta:
model = MovieSession
Expand All @@ -68,13 +78,94 @@ class Meta:
"movie_title",
"cinema_hall_name",
"cinema_hall_capacity",
"tickets_available"
)

def get_tickets_available(self, obj):
total_capacity = obj.cinema_hall.capacity
taken_tickets = Ticket.objects.filter(movie_session=obj).count()
return total_capacity - taken_tickets


class MovieSessionDetailSerializer(MovieSessionSerializer):
movie = MovieListSerializer(many=False, read_only=True)
cinema_hall = CinemaHallSerializer(many=False, read_only=True)
taken_places = serializers.SerializerMethodField()

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):
class Meta:
model = Ticket
fields = (
"id",
"row",
"seat",
"movie_session",
)

def validate(self, attrs):
Ticket.validate_ticket(
row=attrs["row"],
seat=attrs["seat"],
movie_session=attrs["movie_session"],
error_to_raise=serializers.ValidationError

Choose a reason for hiding this comment

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

The use of error_to_raise in the validate method is unconventional. Typically, validation methods directly raise serializers.ValidationError without passing an error class as a parameter. Consider removing this parameter and directly raising serializers.ValidationError.

)
return attrs


class TicketListSerializer(TicketSerializer):
movie_session = MovieSessionListSerializer(many=False, read_only=True)

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


class TicketCreateSerializer(TicketSerializer):
movie_session = serializers.PrimaryKeyRelatedField(
queryset=MovieSession.objects.all()
)


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

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

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)

Choose a reason for hiding this comment

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

Ensure that the creation of Ticket instances respects all validation constraints, including those defined in the Ticket model's clean method. This will help maintain data integrity within the transaction.

return order


class OrderListSerializer(OrderSerializer):
tickets = TicketListSerializer(many=True, read_only=True)

class Meta:
model = Order
fields = (
"id",
"tickets",
"created_at"
)
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
67 changes: 65 additions & 2 deletions cinema/views.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
from rest_framework import viewsets

from cinema.models import Genre, Actor, CinemaHall, Movie, MovieSession
from rest_framework.pagination import PageNumberPagination

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

from cinema.serializers import (
GenreSerializer,
Expand All @@ -12,9 +19,16 @@
MovieDetailSerializer,
MovieSessionDetailSerializer,
MovieListSerializer,
OrderSerializer, OrderListSerializer,
)


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


class GenreViewSet(viewsets.ModelViewSet):
queryset = Genre.objects.all()
serializer_class = GenreSerializer
Expand All @@ -34,6 +48,25 @@ class MovieViewSet(viewsets.ModelViewSet):
queryset = Movie.objects.all()
serializer_class = MovieSerializer

def get_queryset(self):
queryset = Movie.objects.all()

genres = self.request.query_params.get("genres", None)
if genres:
genres_ids = [int(str_id) for str_id in genres.split(",")]

Choose a reason for hiding this comment

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

Consider adding error handling for the conversion of genres query parameter to integers. If a non-integer value is passed, it will raise a ValueError. You might want to handle this exception to avoid server errors.

queryset = queryset.filter(genres__id__in=genres_ids)

actors = self.request.query_params.get("actors", None)
if actors:
actors_ids = [int(str_id) for str_id in actors.split(",")]

Choose a reason for hiding this comment

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

Consider adding error handling for the conversion of actors query parameter to integers. If a non-integer value is passed, it will raise a ValueError. You might want to handle this exception to avoid server errors.

queryset = queryset.filter(actors__id__in=actors_ids)

title = self.request.query_params.get("title", None)
if title:
queryset = queryset.filter(title__icontains=title)

return queryset.distinct()

def get_serializer_class(self):
if self.action == "list":
return MovieListSerializer
Expand All @@ -48,6 +81,19 @@ class MovieSessionViewSet(viewsets.ModelViewSet):
queryset = MovieSession.objects.all()
serializer_class = MovieSessionSerializer

def get_queryset(self):
queryset = MovieSession.objects.all()

date = self.request.query_params.get("date", None)
if date:
queryset = queryset.filter(show_time__date=date)

movie_id = self.request.query_params.get("movie", None)
if movie_id:
queryset = queryset.filter(movie__id=movie_id)

return queryset.distinct()

def get_serializer_class(self):
if self.action == "list":
return MovieSessionListSerializer
Expand All @@ -56,3 +102,20 @@ def get_serializer_class(self):
return MovieSessionDetailSerializer

return MovieSessionSerializer


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

def get_queryset(self):
return self.queryset.filter(user=self.request.user)

Choose a reason for hiding this comment

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

The get_queryset method assumes that the user is authenticated. Ensure that this viewset is protected by authentication and permission classes to prevent unauthorized access.


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

def get_serializer_class(self):
if self.action in ["list", "retrieve"]:
return OrderListSerializer
return OrderSerializer
4 changes: 2 additions & 2 deletions cinema_service/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,11 @@

LANGUAGE_CODE = "en-us"

TIME_ZONE = "UTC"
TIME_ZONE = "Europe/Lisbon"

USE_I18N = True

USE_TZ = False
USE_TZ = True


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