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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Generated by Django 4.1 on 2024-09-27 17:23

from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
('cinema', '0001_initial'),
]

operations = [
migrations.CreateModel(
name='Actor',
fields=[
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('first_name', models.CharField(max_length=255)),
('last_name', models.CharField(max_length=255)),
],
),
migrations.CreateModel(
name='CinemaHall',
fields=[
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('name', models.CharField(max_length=255)),
('rows', models.IntegerField()),
('seats_in_row', models.IntegerField()),
],
),
migrations.CreateModel(
name='Genre',
fields=[
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('name', models.CharField(max_length=255, unique=True)),
],
),
migrations.AddField(
model_name='movie',
name='actors',
field=models.ForeignKey(default='', on_delete=django.db.models.deletion.CASCADE, related_name='movies', to='cinema.actor'),
preserve_default=False,
),
migrations.AddField(
model_name='movie',
name='genres',
field=models.ForeignKey(default='', on_delete=django.db.models.deletion.CASCADE, related_name='movies', to='cinema.genre'),
preserve_default=False,
),
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Generated by Django 4.1 on 2024-09-27 17:56

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('cinema', '0002_actor_cinemahall_genre_movie_actors_movie_genres'),
]

operations = [
migrations.RemoveField(
model_name='movie',
name='actors',
),
migrations.RemoveField(
model_name='movie',
name='genres',
),
migrations.AddField(
model_name='movie',
name='actors',
field=models.ManyToManyField(related_name='movies', to='cinema.actor'),
),
migrations.AddField(
model_name='movie',
name='genres',
field=models.ManyToManyField(related_name='movies', to='cinema.genre'),
),
]
26 changes: 26 additions & 0 deletions cinema/models.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,36 @@
from django.db import models


class Genre(models.Model):
name = models.CharField(max_length=255, unique=True)

def __str__(self) -> str:
return self.name


class Actor(models.Model):
first_name = models.CharField(max_length=255)
last_name = models.CharField(max_length=255)

def __str__(self) -> str:
return f"{self.first_name} {self.last_name}"


class CinemaHall(models.Model):
name = models.CharField(max_length=255)
rows = models.IntegerField()
seats_in_row = models.IntegerField()

def __str__(self) -> str:
return f"{self.name}: rows - {self.rows}, seats - {self.seats_in_row}"


class Movie(models.Model):
title = models.CharField(max_length=255)
description = models.TextField()
duration = models.IntegerField()

Choose a reason for hiding this comment

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

Using IntegerField for movie duration might work, but it's unclear what unit of time you're storing (minutes, seconds, etc.). It might be beneficial to add a help_text attribute or use DurationField for clarity and flexibility.

actors = models.ManyToManyField(Actor, related_name="movies")
genres = models.ManyToManyField(Genre, related_name="movies")

def __str__(self):
return self.title
47 changes: 43 additions & 4 deletions cinema/serializers.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,63 @@
from rest_framework import serializers

from cinema.models import Movie
from cinema.models import Movie, Actor, Genre, CinemaHall

Choose a reason for hiding this comment

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

If importing more than two items, it is better to write them on separate lines. This improves readability and maintenance of the code.

Example:

From taxi.models import (
Driver,
Manufacturer,
Car
)
Read more about topic here



class MovieSerializer(serializers.Serializer):

Choose a reason for hiding this comment

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

The MovieSerializer could be simplified by using ModelSerializer instead of Serializer. Since you're working with a Django model, ModelSerializer automatically handles common functionality (like field definitions and create/update methods) for you.

id = serializers.IntegerField(read_only=True)
title = serializers.CharField(max_length=255)
description = serializers.CharField()
duration = serializers.IntegerField()
actors = serializers.PrimaryKeyRelatedField(
queryset=Actor.objects.all(),
many=True,
)
genres = serializers.PrimaryKeyRelatedField(
queryset=Genre.objects.all(),
many=True,
)

def create(self, validated_data):
return Movie.objects.create(**validated_data)
def create(self, validated_data: dict) -> Movie:
actors = validated_data.pop("actors", [])
genres = validated_data.pop("genres", [])
movie = Movie.objects.create(**validated_data)
movie.actors.set(actors)
movie.genres.set(genres)
return movie

def update(self, instance, validated_data):
def update(self, instance: Movie, validated_data: dict) -> Movie:
instance.title = validated_data.get("title", instance.title)
instance.description = validated_data.get(
"description", instance.description
)
instance.duration = validated_data.get("duration", instance.duration)
actors = validated_data.get("actors", None)
genres = validated_data.get("genres", None)

Choose a reason for hiding this comment

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

In the update method of MovieSerializer, the checks for actors and genres could be simplified. Instead of checking whether they exist and updating only if provided, you might directly set these values, as .set() works with empty lists as well.

if actors:
instance.actors.set(actors)

if genres:
instance.genres.set(genres)

instance.save()

return instance


class ActorSerializer(serializers.ModelSerializer):
class Meta:
model = Actor
fields = ["first_name", "last_name"]


class GenreSerializer(serializers.ModelSerializer):
class Meta:
model = Genre
fields = ["name"]


class CinemaHallSerializer(serializers.ModelSerializer):
class Meta:
model = CinemaHall
fields = ["name", "rows", "seats_in_row"]
46 changes: 42 additions & 4 deletions cinema/urls.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,48 @@
from django.urls import path
from django.urls import path, include
from rest_framework import routers

from cinema.views import movie_list, movie_detail
from cinema.views import (
MovieViewSet,
GenreList,
GenreDetail,
ActorList,
ActorDetail,
CinemaHallViewSet,
)

router = routers.DefaultRouter()

Choose a reason for hiding this comment

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

2 blank lines after imports


router.register("movies", MovieViewSet)

cinema_hall_list = CinemaHallViewSet.as_view(
actions={
"get": "list",
"post": "create",
}
)

cinema_hall_detail = CinemaHallViewSet.as_view(
actions={
"get": "retrieve",
"put": "update",
"patch": "partial_update",
"delete": "destroy",
}
)

urlpatterns = [
path("movies/", movie_list, name="movie-list"),
path("movies/<int:pk>/", movie_detail, name="movie-detail"),
path("", include(router.urls)),
path("actors/", ActorList.as_view(), name="actors-list"),
path("actors/<int:pk>/", ActorDetail.as_view(), name="actors-detail"),
path("genres/", GenreList.as_view(), name="genres-list"),
path("genres/<int:pk>/", GenreDetail.as_view(), name="genres-detail"),
path("cinema_halls/", cinema_hall_list, name="cinema_halls-list"),
path(
"cinema_halls/<int:pk>/",
cinema_hall_detail,
name="cinema_halls-detail"
),

]

app_name = "cinema"
124 changes: 95 additions & 29 deletions cinema/views.py
Original file line number Diff line number Diff line change
@@ -1,45 +1,111 @@
from rest_framework.decorators import api_view
from django.core.serializers import serialize
from django.http import HttpRequest
from rest_framework.response import Response
from rest_framework import status
from rest_framework import status, viewsets, mixins, generics

from django.shortcuts import get_object_or_404
from rest_framework.views import APIView

from cinema.models import Movie
from cinema.serializers import MovieSerializer
from cinema.models import Movie, Genre, Actor, CinemaHall
from cinema.serializers import (
MovieSerializer,
GenreSerializer,
ActorSerializer,
CinemaHallSerializer,
)


@api_view(["GET", "POST"])
def movie_list(request):
if request.method == "GET":
movies = Movie.objects.all()
serializer = MovieSerializer(movies, many=True)
class GenreList(APIView):
def get(self, request: HttpRequest) -> Response:
genre = Genre.objects.all()
serializer = GenreSerializer(genre, many=True)
return Response(serializer.data, status=status.HTTP_200_OK)

if request.method == "POST":
serializer = MovieSerializer(data=request.data)
if serializer.is_valid():
serializer.save()
return Response(serializer.data, status=status.HTTP_201_CREATED)
def post(self, request: HttpRequest) -> Response:
serializer = GenreSerializer(data=request.data)
serializer.is_valid(raise_exception=True)
serializer.save()
return Response(serializer.data, status=status.HTTP_201_CREATED)

return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)

class GenreDetail(APIView):
def get_object(self, pk: int) -> Genre:
return get_object_or_404(Genre, pk=pk)

@api_view(["GET", "PUT", "DELETE"])
def movie_detail(request, pk):
movie = get_object_or_404(Movie, pk=pk)

if request.method == "GET":
serializer = MovieSerializer(movie)
def get(self, request: HttpRequest, pk: int) -> Response:
genre = self.get_object(pk)
serializer = GenreSerializer(genre)
return Response(serializer.data, status=status.HTTP_200_OK)

if request.method == "PUT":
serializer = MovieSerializer(movie, data=request.data)
if serializer.is_valid():
serializer.save()
return Response(serializer.data, status=status.HTTP_200_OK)
def put(self, request: HttpRequest, pk: int) -> Response:
genre = self.get_object(pk)
serializer = GenreSerializer(genre, data=request.data)
serializer.is_valid(raise_exception=True)
serializer.save()
return Response(serializer.data)

return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
def patch(self, request: HttpRequest, pk: int) -> Response:
genre = self.get_object(pk)
serializer = GenreSerializer(genre, data=request.data, partial=True)
serializer.is_valid(raise_exception=True)
serializer.save()
return Response(serializer.data)

if request.method == "DELETE":
movie.delete()
def delete(self, request: HttpRequest, pk: int) -> Response:
genre = self.get_object(pk)
genre.delete()
return Response(status=status.HTTP_204_NO_CONTENT)


class ActorList(
mixins.ListModelMixin,
mixins.CreateModelMixin,
generics.GenericAPIView
):
queryset = Actor.objects.all()
serializer_class = ActorSerializer

def get(self, request: HttpRequest, *args, **kwargs) -> Response:
return self.list(request, *args, **kwargs)

def post(self, request: HttpRequest, *args, **kwargs) -> Response:
return self.create(request, *args, **kwargs)

Choose a reason for hiding this comment

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

In your ActorList and ActorDetail classes, the get and post methods could be simplified further by calling the methods directly.

Copy link
Author

Choose a reason for hiding this comment

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

I cannot delete these methods because it will cause test failures in both classes

Copy link
Author

Choose a reason for hiding this comment

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

OK. I've found the solution for this already. I have changed inherit, but in READMY.md write that for the Actor model I need to use a GenericAPIView


class ActorDetail(
mixins.RetrieveModelMixin,
mixins.UpdateModelMixin,
mixins.DestroyModelMixin,
generics.GenericAPIView,
):
queryset = Actor.objects.all()
serializer_class = ActorSerializer

def get(self, request: HttpRequest, *args, **kwargs) -> Response:
return self.retrieve(request, *args, **kwargs)

def put(self, request: HttpRequest, *args, **kwargs) -> Response:
return self.update(request, *args, **kwargs)

def patch(self, request: HttpRequest, *args, **kwargs) -> Response:
return self.partial_update(request, *args, **kwargs)

def delete(self, request: HttpRequest, *args, **kwargs) -> Response:
return self.destroy(request, *args, **kwargs)

Choose a reason for hiding this comment

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

No need for overrides:
Since ListModelMixin provides the logic for listing and creating objects, and RetrieveModelMixin, UpdateModelMixin, and DestroyModelMixin handle retrieval, updating, and deletion, you don't need to redefine those methods.


class CinemaHallViewSet(
mixins.ListModelMixin,
mixins.CreateModelMixin,
mixins.RetrieveModelMixin,
mixins.UpdateModelMixin,
mixins.DestroyModelMixin,
viewsets.GenericViewSet
):
queryset = CinemaHall.objects.all()
serializer_class = CinemaHallSerializer


class MovieViewSet(viewsets.ModelViewSet):
queryset = Movie.objects.all()
serializer_class = MovieSerializer
Loading