Solution #870
Solution #870
Conversation
db/models.py
Outdated
| class Order(models.Model): | ||
| created_at = models.DateTimeField(auto_now_add=True) | ||
| user = models.ForeignKey( | ||
| "User", |
There was a problem hiding this comment.
you should use AUTH USER MODEL from settings
| return f"{self.created_at}" | ||
|
|
||
| class Meta: | ||
| ordering = ("-created_at",) |
There was a problem hiding this comment.
Meta class should be defined right after fields by django convention
db/models.py
Outdated
| ) | ||
|
|
||
| def clean(self) -> None: | ||
| if self.row > self.movie_session.cinema_hall.rows: |
There was a problem hiding this comment.
you don't validate if numbers are below zero here
There was a problem hiding this comment.
It is possible to combine these two conditions above one Validation Error such as : if not 0 < x <= y
db/models.py
Outdated
| constraints = [ | ||
| models.UniqueConstraint( | ||
| fields=["movie_session", "row", "seat"], | ||
| name="unique_ticket_constraint" |
There was a problem hiding this comment.
constraint name should contain names of fields that are unique + Meta class should be moved above str method
services/movie_session.py
Outdated
| {"row": ticket.row, "seat": ticket.seat} | ||
| for ticket in tickets | ||
| ] | ||
| return taken_seats |
There was a problem hiding this comment.
can you do it using .values() on queryset?
services/order.py
Outdated
| id=ticket_data["movie_session"] | ||
| ) | ||
| Ticket.objects.create( | ||
| movie_session=movie_session, |
There was a problem hiding this comment.
no need to create movie_session object - you can pass an id on Ticket creation instead
services/order.py
Outdated
| if username: | ||
| user = User.objects.get(username=username) | ||
| orders = Order.objects.filter(user=user) | ||
| else: |
There was a problem hiding this comment.
refactor so you use only one if statement and return intruction. Use lookup to filter data user__username
services/order.py
Outdated
| return order | ||
|
|
||
|
|
||
| def get_orders(username: str = None) -> QuerySet: |
There was a problem hiding this comment.
Add annotation for optional parameters
services/user.py
Outdated
| def create_user( | ||
| username: str, | ||
| password: str, | ||
| email: str = None, |
services/user.py
Outdated
| first_name: str = None, | ||
| last_name: str = None | ||
| ) -> User: | ||
| user = User.objects.get(id=user_id) |
There was a problem hiding this comment.
reuse your get_user function here to make it DRY
services/movie.py
Outdated
|
|
||
| from db.models import Movie | ||
|
|
||
|
|
||
| def get_movies( | ||
| genres_ids: list[int] = None, | ||
| actors_ids: list[int] = None, | ||
| title: str = None |
There was a problem hiding this comment.
Annotation that this parameter can be None is missing
db/models.py
Outdated
| ) | ||
|
|
||
| def clean(self) -> None: | ||
| if self.row > self.movie_session.cinema_hall.rows: |
There was a problem hiding this comment.
It is possible to combine these two conditions above one Validation Error such as : if not 0 < x <= y
services/movie_session.py
Outdated
|
|
||
|
|
||
| def get_taken_seats(movie_session_id: int) -> list[dict]: | ||
| tickets = ( |
There was a problem hiding this comment.
no need to store tickets in a variable that you will not reuse. just return result
| username: str, | ||
| date: str = None | ||
| ) -> Order: | ||
| user = User.objects.get(username=username) |
There was a problem hiding this comment.
yeah, my mistake there is no need for transaction in read operation here 😊
services/movie.py
Outdated
| @@ -38,3 +40,7 @@ def create_movie( | |||
| movie.actors.set(actors_ids) | |||
|
|
|||
| return movie | |||
There was a problem hiding this comment.
You don't need return anything in create/update/delete functions, check everywhere
| movie_session = models.ForeignKey( | ||
| to=MovieSession, | ||
| on_delete=models.CASCADE, | ||
| related_name="tickets") | ||
| order = models.ForeignKey( | ||
| to=Order, | ||
| on_delete=models.CASCADE, | ||
| related_name="tickets") |
There was a problem hiding this comment.
the closing bracket should be on a separate line
No description provided.