Solution Politov #680

Alexandr-Politov · 2024-09-27T14:56:20Z

No description provided.

BastovOleksandr · 2024-09-28T21:50:33Z

cinema/serializers.py

+class ActorSerializer(serializers.Serializer):
+    id = serializers.IntegerField(read_only=True)
+    first_name = serializers.CharField(max_length=50)
+    last_name = serializers.CharField(max_length=50)


I'm not sure, but I think it is time to use serializers.ModelSerializer instead of Serializer
so your "serializers.py" code will become much shorter
4 strings almost for each class, except Movie with it ManyToMany realations

BastovOleksandr · 2024-09-28T21:57:33Z

cinema/views.py

+class GenreList(views.APIView):
+    def get(self, request):
+        genres = Genre.objects.all()
+        serializer = GenreSerializer(genres, many=True)


I got a reject for not using parameter type annotations, including what the method returns

BastovOleksandr · 2024-09-28T22:01:39Z

cinema/views.py

+class ActorList(
+    generics.GenericAPIView,
+    mixins.ListModelMixin,
+    mixins.CreateModelMixin


order is matter
mixins are going first

BastovOleksandr · 2024-09-28T22:01:57Z

cinema/views.py

+class ActorDetail(
+    generics.GenericAPIView,
+    mixins.RetrieveModelMixin,
+    mixins.UpdateModelMixin,


same here, just fix it everywhere

BastovOleksandr

I double checked, it seems like it's time to use serializers.ModelSerializer

Arsen-hrynevych

You've duplicated a lot of code that's already implemented in Django. To adhere to the DRY (Don't Repeat Yourself) principle, you could simplify your code.

Arsen-hrynevych · 2024-09-29T15:46:24Z

cinema/serializers.py

+# class MovieSerializer(serializers.Serializer):
+#     id = serializers.IntegerField(read_only=True)
+#     title = serializers.CharField(max_length=255)
+#     description = serializers.CharField()
+#     duration = serializers.IntegerField()
+#     actors = serializers.PrimaryKeyRelatedField(
+#         many=True, queryset=Actor.objects.all()
+#     )
+#     genres = serializers.PrimaryKeyRelatedField(
+#         many=True, queryset=Genre.objects.all()
+#     )
+#
+#     def create(self, validated_data):
+#         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):
+#         instance.title = validated_data.get("title", instance.title)
+#         instance.description = validated_data.get(
+#             "description", instance.description
+#         )
+#         instance.duration = validated_data.get("duration", instance.duration)
+#         if "actors" in validated_data:
+#             actors = validated_data.pop("actors")
+#             instance.actors.set(actors)
+#         if "genres" in validated_data:
+#             genres = validated_data.pop("genres")
+#             instance.genres.set(genres)
+#         instance.save()
+#         return instance


Commented-out code should not be included in the repository.

Arsen-hrynevych · 2024-09-29T15:46:36Z

cinema/urls.py

+    CinemaHallViewSet,
+    MovieViewSet,
+)
+


2 blank lines after imports

Arsen-hrynevych · 2024-09-29T15:52:29Z

cinema/views.py

+
+    def get(self, request: Request, *args, **kwargs) -> Response:
+        return self.retrieve(request, *args, **kwargs)
+
+    def put(self, request: Request, *args, **kwargs) -> Response:
+        return self.update(request, *args, **kwargs)
+
+    def patch(self, request: Request, *args, **kwargs) -> Response:
+        return self.update(request, partial=True, *args, **kwargs)
+
+    def delete(self, request: Request, *args, **kwargs) -> Response:
+        return self.destroy(request, *args, **kwargs)


Mixins already take care of this for you, making the code redundant.

No. Mixins take care of this methods only with GenericViewSet. With GenericAPIView they only let you use this simplified srtucture.

@Alexandr-Politov actually for both generic classes you should use mixins(you are correct!). Mixins define actions (list, create, destroy,..) why Viewset and View have (get, post, delete,...) methods. But they aren't connected. By default DRF doesn't know that for "def post" it should call "create" action. This is why we need to do it explicitly.
Or we can inherit from ModelViewset/ListCreateAPIView/RetrieveUpdateDestroyView that already have connected methods with actions. The best idea it to view DRF code by opening these classes and check their implementation ;)

MNDonut · 2024-09-30T16:26:39Z

cinema/serializers.py

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

-    def create(self, validated_data):
-        return Movie.objects.create(**validated_data)

-    def update(self, instance, validated_data):
-        instance.title = validated_data.get("title", instance.title)
-        instance.description = validated_data.get(
-            "description", instance.description
-        )
-        instance.duration = validated_data.get("duration", instance.duration)
+class GenreSerializer(serializers.ModelSerializer):
+    class Meta:
+        model = Genre
+        fields = ("id", "name",)

-        instance.save()

-        return instance
+class CinemaHallSerializer(serializers.ModelSerializer):
+    class Meta:
+        model = CinemaHall
+        fields = ("id", "name", "rows", "seats_in_row")
+
+
+class MovieSerializer(serializers.ModelSerializer):
+    actors = serializers.PrimaryKeyRelatedField(
+        many=True, queryset=Actor.objects.all()
+    )
+    genres = serializers.PrimaryKeyRelatedField(
+        many=True, queryset=Genre.objects.all()
+    )
+
+    class Meta:
+        model = Movie


We'll learn these serializers in API Serilizer topic, for this you should still use Serializer because

MNDonut · 2024-09-30T16:27:07Z

cinema/urls.py

+)
+


2 blank lines after the last import

MNDonut · 2024-09-30T16:39:32Z

cinema/views.py

+
+    def get(self, request: Request, *args, **kwargs) -> Response:
+        return self.retrieve(request, *args, **kwargs)
+
+    def put(self, request: Request, *args, **kwargs) -> Response:
+        return self.update(request, *args, **kwargs)
+
+    def patch(self, request: Request, *args, **kwargs) -> Response:
+        return self.update(request, partial=True, *args, **kwargs)
+
+    def delete(self, request: Request, *args, **kwargs) -> Response:
+        return self.destroy(request, *args, **kwargs)


@Alexandr-Politov actually for both generic classes you should use mixins(you are correct!). Mixins define actions (list, create, destroy,..) why Viewset and View have (get, post, delete,...) methods. But they aren't connected. By default DRF doesn't know that for "def post" it should call "create" action. This is why we need to do it explicitly.
Or we can inherit from ModelViewset/ListCreateAPIView/RetrieveUpdateDestroyView that already have connected methods with actions. The best idea it to view DRF code by opening these classes and check their implementation ;)

Oleksl888 · 2024-10-02T05:37:25Z

cinema/views.py

+        serializer = GenreSerializer(data=request.data)
+        serializer.is_valid(raise_exception=True)
+        serializer.save()
+        return Response(serializer.data, status=status.HTTP_201_CREATED)


In this task it actually asks to

Don't forget return Response with errors if serializer is not valid:

Oleksl888

Good job!

Alexandr-Politov added 7 commits September 27, 2024 16:26

models created

cb4b3d9

migrations

678f444

serializers created

dae8b8d

views and urls created

e0df63b

flake8 mistakes

c05f588

fixed according to tests

73b150d

urls.py flake8

5f3101c

BastovOleksandr reviewed Sep 28, 2024

View reviewed changes

BastovOleksandr suggested changes Sep 28, 2024

View reviewed changes

Alexandr-Politov added 4 commits September 29, 2024 10:49

type annotations added to vies.py

7495376

inheritance order fixed for mixins > viewsets in views.py

7d5ea31

serializers' inheritance changed from Serializer to ModelSerializer

1b74045

flake8

fef641f

Alexandr-Politov requested a review from BastovOleksandr September 29, 2024 08:26

BastovOleksandr approved these changes Sep 29, 2024

View reviewed changes

Arsen-hrynevych suggested changes Sep 29, 2024

View reviewed changes

Alexandr-Politov added 2 commits September 29, 2024 21:37

Commented-out code deleted

3bccbd8

2 blank lines after imports in urls.py

4f3694e

Alexandr-Politov requested a review from Arsen-hrynevych September 29, 2024 18:49

MNDonut suggested changes Sep 30, 2024

View reviewed changes

Alexandr-Politov added 2 commits October 1, 2024 17:38

2 blank lines after imports in both urls.py

8a4b123

serializers changed back into serializers.Serializer

008cfb6

Alexandr-Politov requested a review from MNDonut October 1, 2024 14:43

Oleksl888 reviewed Oct 2, 2024

View reviewed changes

Oleksl888 approved these changes Oct 2, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Solution Politov #680

Solution Politov #680

Alexandr-Politov commented Sep 27, 2024

BastovOleksandr Sep 28, 2024

BastovOleksandr Sep 28, 2024

BastovOleksandr Sep 28, 2024

BastovOleksandr Sep 28, 2024

BastovOleksandr left a comment

Arsen-hrynevych left a comment

Arsen-hrynevych Sep 29, 2024

Arsen-hrynevych Sep 29, 2024

Arsen-hrynevych Sep 29, 2024

Alexandr-Politov Sep 29, 2024

MNDonut Sep 30, 2024 •

edited

Loading

MNDonut Sep 30, 2024

MNDonut Sep 30, 2024

MNDonut Sep 30, 2024 •

edited

Loading

Oleksl888 Oct 2, 2024

Oleksl888 left a comment

Solution Politov #680

Are you sure you want to change the base?

Solution Politov #680

Conversation

Alexandr-Politov commented Sep 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BastovOleksandr left a comment

Choose a reason for hiding this comment

Arsen-hrynevych left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MNDonut Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MNDonut Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Oleksl888 left a comment

Choose a reason for hiding this comment

MNDonut Sep 30, 2024 •

edited

Loading

MNDonut Sep 30, 2024 •

edited

Loading