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

first_try #646

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

first_try #646

wants to merge 6 commits into from

Conversation

yashkunn
Copy link

@yashkunn yashkunn commented Oct 1, 2024

No description provided.

@@ -14,6 +14,7 @@ def setUp(self):

def test_get_actors(self):
response = self.client.get("/api/cinema/actors/")
print(response.data)

Choose a reason for hiding this comment

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

Don't forget to delete testing prints

cinema/urls.py Outdated
@@ -6,7 +6,7 @@
ActorViewSet,
CinemaHallViewSet,
MovieViewSet,
MovieSessionViewSet,
MovieSessionViewSet, OrderViewSet,

Choose a reason for hiding this comment

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

Move to new line

Choose a reason for hiding this comment

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

Good mark

cinema/views.py Outdated
@@ -11,7 +14,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.

same

Choose a reason for hiding this comment

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

Revert test files

Comment on lines 86 to 87
movie = MovieListSerializer(many=False, read_only=True)
cinema_hall = CinemaHallSerializer(many=False, read_only=True)

Choose a reason for hiding this comment

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

many be default is False

order = Order.objects.create(**validated_data)
for ticket_data in tickets_data:
Ticket.objects.create(order=order, **ticket_data)
return order

Choose a reason for hiding this comment

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

In create/update/delete fucntion you don't need return anything

cinema/urls.py Outdated
@@ -6,7 +6,7 @@
ActorViewSet,
CinemaHallViewSet,
MovieViewSet,
MovieSessionViewSet,
MovieSessionViewSet, OrderViewSet,

Choose a reason for hiding this comment

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

Good mark

cinema/utils.py Outdated
@@ -0,0 +1,2 @@
def filter_request_to_int(query_params):

Choose a reason for hiding this comment

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

Annotations, everywhere :) They still exist)

cinema/views.py Outdated
@@ -43,6 +46,28 @@ def get_serializer_class(self):

return MovieSerializer

def get_queryset(self):
queryset = self.queryset

Choose a reason for hiding this comment

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

call super method instead of accesing this attribute directly
solve N+1 problem

cinema/views.py Outdated

if actors:
actors = filter_request_to_int(actors)
queryset = queryset.filter(actors__id__in=actors).distinct()

Choose a reason for hiding this comment

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

Move distinct to return

cinema/views.py Outdated
@@ -56,3 +81,53 @@ def get_serializer_class(self):
return MovieSessionDetailSerializer

return MovieSessionSerializer

def get_queryset(self):
queryset = self.queryset

Choose a reason for hiding this comment

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

Same here

cinema/models.py Outdated
force_update=False,
using=None,
update_fields=None,
self,

Choose a reason for hiding this comment

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

fix indentation everywhere

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

Choose a reason for hiding this comment

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

Here better to use nested serializer instead of creating method. Create a serializer that has row and seat fields and pass it here

db.sqlite3 Outdated

Choose a reason for hiding this comment

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

remove db from remote

Copy link

@Oleksl888 Oleksl888 left a comment

Choose a reason for hiding this comment

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

All good - but please delete DB from remote

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants