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

implemented tickets and orders architecture #829

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

Conversation

MasakDirt
Copy link

No description provided.

db/models.py Outdated
from django.db import models
from django.db.models import UniqueConstraint

import settings
Copy link

Choose a reason for hiding this comment

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

it's better to use
from django.conf import settings that will import django settings because you may have a folder named "settings"

db/models.py Outdated
Comment on lines 92 to 94
to=MovieSession, on_delete=models.CASCADE, related_name="tickets")
order = models.ForeignKey(
to=Order, on_delete=models.CASCADE, related_name="tickets")
Copy link

Choose a reason for hiding this comment

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

we don't add multiple parameters on the same line.
Either they are on the same lvl that ( bracket or 1 parameter - 1 line

Comment on lines 8 to 10
genres_ids: list[int] = None,
actors_ids: list[int] = None,
title: str = None,
Copy link

Choose a reason for hiding this comment

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

you didn't annotate None type

Comment on lines 33 to 34
genres_ids: list = None,
actors_ids: list = None,
Copy link

Choose a reason for hiding this comment

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

fix annotation

Comment on lines 30 to 32
show_time: str = None,
movie_id: int = None,
cinema_hall_id: int = None,
Copy link

Choose a reason for hiding this comment

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

annotation

Comment on lines 24 to 26
Ticket.objects.create(row=ticket["row"], seat=ticket["seat"],
movie_session_id=ticket["movie_session"],
order=order)
Copy link

Choose a reason for hiding this comment

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

fix formatting. parameters should have the same indentation

services/user.py Outdated
Comment on lines 16 to 19
if first_name:
user.first_name = first_name
else:
user.first_name = ""
Copy link

Choose a reason for hiding this comment

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

or user.first_name = first_name or ""

services/user.py Outdated
Comment on lines 10 to 12
email: str = None,
first_name: str = None,
last_name: str = None,
Copy link

Choose a reason for hiding this comment

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

fix annotation

services/user.py Outdated
Comment on lines 46 to 50
username: str = None,
password: str = None,
email: str = None,
first_name: str = None,
last_name: str = None,
Copy link

Choose a reason for hiding this comment

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

fix annotation

@MasakDirt MasakDirt requested a review from MNDonut August 31, 2024 08:15
if genres_ids:
movie.genres.set(genres_ids)
if actors_ids:
movie.actors.set(actors_ids)

return movie

Choose a reason for hiding this comment

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

You don’t need to return anything in create/update/delete methods, check everywhere

actors_ids: list[int] = None,
genres_ids: list[int] | None = None,
actors_ids: list[int] | None = None,
title: str | None = None,
) -> QuerySet:
Copy link

Choose a reason for hiding this comment

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

specify what kind of data this queryset contains

show_time=movie_show_time,
movie_id=movie_id,
cinema_hall_id=cinema_hall_id,
)


def get_movies_sessions(session_date: str = None) -> QuerySet:
def get_movies_sessions(session_date: str | None = None) -> QuerySet:
Copy link

Choose a reason for hiding this comment

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

specify what kind of data this queryset contains

@@ -42,3 +45,9 @@ def update_movie_session(

def delete_movie_session_by_id(session_id: int) -> None:
MovieSession.objects.get(id=session_id).delete()
Copy link

Choose a reason for hiding this comment

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

follow DRY and use get_movie_sessions

def create_order(
tickets: list[dict],
username: str,
date: str = None
Copy link

Choose a reason for hiding this comment

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

str or None

)


def get_orders(username: str = None) -> QuerySet[Order]:
Copy link

Choose a reason for hiding this comment

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

str or None

services/user.py Outdated
Comment on lines 54 to 56
with transaction.atomic():
if password:
user.set_password(password)
Copy link

Choose a reason for hiding this comment

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

do you think only password can cause an error?
What if you get incorrect email like someemail.com?
Or the length of first_name will be > than max_length of this field?

services/user.py Outdated
Comment on lines 19 to 21
with transaction.atomic():
user.set_password(password)
user.save()
Copy link

Choose a reason for hiding this comment

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

do you think only password can cause an error?
What if you get incorrect email like someemail.com?
Or the length of first_name will be > than max_length of this field?

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.

3 participants