solution #864
solution #864
Conversation
db/models.py
Outdated
| created_at = models.DateTimeField(auto_now_add=True) | ||
| user = models.ForeignKey( | ||
| settings.AUTH_USER_MODEL, | ||
| on_delete=models.CASCADE |
db/models.py
Outdated
| movie_session = models.ForeignKey( | ||
| to=MovieSession, | ||
| on_delete=models.CASCADE, | ||
| related_name="movie_sessions" |
db/models.py
Outdated
| order = models.ForeignKey( | ||
| to=Order, | ||
| on_delete=models.CASCADE, | ||
| related_name="orders" |
There was a problem hiding this comment.
related name is how tickets will be accessed from Order model. Please correct
db/models.py
Outdated
| max_rows = self.movie_session.cinema_hall.rows | ||
| max_seats = self.movie_session.cinema_hall.seats_in_row | ||
|
|
||
| if max_rows < self.row: |
There was a problem hiding this comment.
this doesn't validate vs negative numbers
db/models.py
Outdated
| constraints = [ | ||
| UniqueConstraint( | ||
| fields=["row", "seat", "movie_session"], | ||
| name="unique_ticket_seat_row_movie_session") |
There was a problem hiding this comment.
well done on naming contraint in a right way 👏
But please move Meta class right under fields definition - it is placed there as a convetion in django
services/order.py
Outdated
| ) | ||
| Ticket.objects.create( | ||
| order=order, | ||
| movie_session=movie_session, |
There was a problem hiding this comment.
you can pass movie_session_id instead of passing an object here
services/user.py
Outdated
|
|
||
|
|
||
| def get_user(user_id: int) -> User: | ||
| return get_user_model().objects.filter(id=user_id).first() |
services/user.py
Outdated
| first_name: str | None = None, | ||
| last_name: str | None = None | ||
| ) -> None: | ||
| user = get_user_model().objects.filter(id=user_id).first() |
There was a problem hiding this comment.
you can reuse your get_user function here
services/user.py
Outdated
| ) -> None: | ||
| user = get_user_model().objects.filter(id=user_id).first() | ||
|
|
||
| if not user: |
There was a problem hiding this comment.
no need for this check - if user doesnt exist let the error happen
settings.py
Outdated
|
|
||
| AUTH_USER_MODEL = "db.User" | ||
|
|
||
| AUTH_PASSWORD_VALIDATORS = [ |
There was a problem hiding this comment.
you don't need to add this in this task
db/models.py
Outdated
| from django.db import models | ||
| from django.db.models import UniqueConstraint | ||
|
|
||
| import settings |
There was a problem hiding this comment.
it's better to use from django.conf import settings because you may have settings folder of another settings file and python will import incorrect settings
db/models.py
Outdated
| related_name="movie_session_tickets" | ||
| ) | ||
| order = models.ForeignKey( | ||
| to=Order, | ||
| on_delete=models.CASCADE, | ||
| related_name="order_tickets" | ||
| ) |
There was a problem hiding this comment.
you overwhelm it too much
You have MovieSession object. For example
my_session = MovieSession.objects.filter(show_date='2024-09-03', movie='Harry Potter').first()
I know it's MovieSession, now I want to get all tickets of this movie
my_session.movie_session_tickets.all() # it's a bit redundant to add "movie_session" in related_name because I know that my_session is MovieSession object
so instead it's better to do
my_session.tickets.all()
Fix everywhere ;)
db/models.py
Outdated
| if max_rows < self.row or self.row < 0: | ||
| raise ValidationError( | ||
| {"row": [f"row number must be in available range: " | ||
| f"(1, rows): (1, {self.row - 1})"]} |
There was a problem hiding this comment.
what if row is 0? 0 < 0 will not return false and raise error but you mention here that row should be at least 1
db/models.py
Outdated
| {"row": [f"row number must be in available range: " | ||
| f"(1, rows): (1, {self.row - 1})"]} | ||
| ) | ||
| if max_seats < self.seat or self.seat < 0: |
services/movie_session.py
Outdated
| def get_taken_seats(movie_session_id: int) -> list[dict]: | ||
| tickets = Ticket.objects.filter(movie_session_id=movie_session_id) | ||
|
|
||
| return [{"row": ticket.row, "seat": ticket.seat} for ticket in tickets] |
services/order.py
Outdated
| def create_order( | ||
| tickets: list[dict], | ||
| username: str, | ||
| date: datetime = None |
services/order.py
Outdated
|
|
||
| def get_orders( | ||
| username: str | None = None, | ||
| ) -> QuerySet: |
There was a problem hiding this comment.
specify what kind of data this queryset contains
services/movie.py
Outdated
| if actors_ids: | ||
| 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
No description provided.