implement py-tickets-and-orders #863
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
|
|
||
|
|
||
| class Ticket(models.Model): | ||
| movie_session = models.ForeignKey(MovieSession, on_delete=models.CASCADE) |
db/models.py
Outdated
| self.full_clean() | ||
| return super().save(*args, **kwargs) | ||
|
|
||
| class Meta: |
There was a problem hiding this comment.
as a convention in django Meta class is defined right after fields definition
services/movie_session.py
Outdated
| movie_session_id=movie_session_id | ||
| ).values("row", "seat") | ||
|
|
||
| return list(row_seats) |
There was a problem hiding this comment.
no need to store row_seats in a variable - just return whole expression in list()
| date: str | None = None | ||
| ) -> Order: | ||
|
|
||
| user = get_user_model().objects.get(username=username) |
There was a problem hiding this comment.
shouldn't this be as well under atomic decorator?
There was a problem hiding this comment.
Lol this actually is a read operation that wouldn't need a rollback in case if something happens :)
But it is actually better to wrap the entire function in transaction
services/user.py
Outdated
| def create_user( | ||
| username: str, | ||
| password: Any, | ||
| email: str = None, |
There was a problem hiding this comment.
add annotation that these params are optional
services/user.py
Outdated
|
|
||
|
|
||
| def get_user(user_id: int) -> User: | ||
| return User.objects.get(id=user_id) |
There was a problem hiding this comment.
use get_user_model() here. Also use pk instead of id
services/user.py
Outdated
| def update_user( | ||
| user_id: int, | ||
| username: str | None = None, | ||
| password: Any | None = None, |
services/user.py
Outdated
|
|
||
| def create_user( | ||
| username: str, | ||
| password: Any, |
services/user.py
Outdated
| first_name: str | None = None, | ||
| last_name: str | None = None | ||
| ) -> None: | ||
| user = get_user_model().objects.get(pk=user_id) |
There was a problem hiding this comment.
you can reuse your get_user function here
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 want to have settings folder or your project has multiple settings files
db/models.py
Outdated
| def save(self, *args, **kwargs) -> Any: | ||
| self.full_clean() | ||
| return super().save(*args, **kwargs) |
There was a problem hiding this comment.
Are you sure it returns it? It returns None actually(you can hover your mouse on super.save and will see which type it should return
services/movie.py
Outdated
| @@ -6,6 +7,7 @@ | |||
| def get_movies( | |||
| genres_ids: list[int] = None, | |||
| actors_ids: list[int] = None, | |||
| title: str = None, | |||
There was a problem hiding this comment.
This haven't been added - you need to add annotation that this parameter can also be None
services/movie_session.py
Outdated
| @@ -42,3 +44,9 @@ def update_movie_session( | |||
|
|
|||
| def delete_movie_session_by_id(session_id: int) -> None: | |||
| MovieSession.objects.get(id=session_id).delete() | |||
settings.py
Outdated
| "db", | ||
| "django.contrib.auth", | ||
| "django.contrib.contenttypes", | ||
| ] |
There was a problem hiding this comment.
actually at the top should be default django apps, then yours
db/models.py
Outdated
| movie_session = models.ForeignKey( | ||
| MovieSession, | ||
| on_delete=models.CASCADE, | ||
| related_name="sessions_tickets" |
There was a problem hiding this comment.
just tickets would be enough here. Use case:
movie_session.tickets
OR
order.tickets
gives you access to Ticket table on corresponding model
services/movie.py
Outdated
| @@ -6,6 +7,7 @@ | |||
| def get_movies( | |||
| genres_ids: list[int] = None, | |||
| actors_ids: list[int] = None, | |||
| title: str = None, | |||
There was a problem hiding this comment.
This haven't been added - you need to add annotation that this parameter can also be None
services/order.py
Outdated
| # with transaction.atomic(): | ||
| order = Order.objects.create(user=user) | ||
| if date: | ||
| order.created_at = parse_datetime(date) |
There was a problem hiding this comment.
you can pass date as is - no need for conversion here. Don't forget to clean up imports as well
No description provided.