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

eeettt5 #530

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

eeettt5 #530

wants to merge 18 commits into from

Conversation

melnikalex1977
Copy link

No description provided.

Choose a reason for hiding this comment

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

Remove this file, and folder

Choose a reason for hiding this comment

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

???

db/models.py Outdated
]

def __str__(self) -> str:
return f"Matrix 2019-08-19 20:30:00 (row: {self.row}, seat: {self.seat})"

Choose a reason for hiding this comment

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

Why matrix and date hardcoded?

Choose a reason for hiding this comment

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

Not fixed

from db.models import Movie


def get_movies(
genres_ids: list[int] = None,
actors_ids: list[int] = None,
title=None,

Choose a reason for hiding this comment

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

add type annotations

@@ -15,13 +16,18 @@ def get_movies(
if actors_ids:
queryset = queryset.filter(actors__id__in=actors_ids)

if title:
# фільтрувати фільми за назвою, якщо аргумент `title` переданий
return Movie.objects.filter(title__icontains=title)

Choose a reason for hiding this comment

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

Why you do return this? And do not use already filtered queryset

tickets = Ticket.objects.filter(movie_session_id=movie_session_id)
for ticket in tickets:
taken_seats.append({"row": ticket.row, "seat": ticket.seat})
return taken_seats

Choose a reason for hiding this comment

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

Add empty line

for ticket_data in tickets:
row = ticket_data.get("row")
seat = ticket_data.get("seat")
movie_session = MovieSession.objects.get(pk=1)

Choose a reason for hiding this comment

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

Why do you use pk=1? Also .get redundant here

Choose a reason for hiding this comment

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

Not fixed why pk=1???

Comment on lines 34 to 37
if username is not None:
return Order.objects.filter(user__username=username).order_by("created_at")
elif username is None:
return Order.objects.all().order_by("-created_at")

Choose a reason for hiding this comment

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

Use just one if here

services/user.py Outdated
user.last_name = last_name
user.save()
return user
except User.DoesNotExist:

Choose a reason for hiding this comment

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

This is not possible

Choose a reason for hiding this comment

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

Remove try-except!!!

services/user.py Outdated
Comment on lines 24 to 26
try:
user = User.objects.get(id=user_id)
except User.DoesNotExist:

Choose a reason for hiding this comment

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

Use get_object_or_404

Choose a reason for hiding this comment

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

Not fixed!

Choose a reason for hiding this comment

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

do it

@melnikalex1977
Copy link
Author

перевір виконання цього завдання по даній адресі #537 (comment)

@melnikalex1977
Copy link
Author

перед тим як закинути на github виконане завдання я не побачив твої зауваження, тому якщо можна я їх виправлю після твоїх зауважень до #537 (comment)

Copy link

@ArsenPidhoretskyi ArsenPidhoretskyi left a comment

Choose a reason for hiding this comment

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

Comments are not fixed

Choose a reason for hiding this comment

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

???

db/models.py Outdated
@@ -49,4 +54,56 @@ class MovieSession(models.Model):
)

def __str__(self) -> str:
return f"{self.movie.title} {str(self.show_time)}"
return f"{self.movie.title} {str(self.show_time)} {self.cinema_hall.rows} {self.cinema_hall.seats_in_row}"

Choose a reason for hiding this comment

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

Suggested change
return f"{self.movie.title} {str(self.show_time)} {self.cinema_hall.rows} {self.cinema_hall.seats_in_row}"
return f"{self.movie.title} {self.show_time} {self.cinema_hall.rows} {self.cinema_hall.seats_in_row}"

db/models.py Outdated
]

def __str__(self) -> str:
return f"Matrix 2019-08-19 20:30:00 (row: {self.row}, seat: {self.seat})"

Choose a reason for hiding this comment

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

Not fixed

db/models.py Outdated
class User(AbstractUser):
USERNAME_FIELD = "username"
REQUIRED_FIELDS = []
pass

Choose a reason for hiding this comment

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

remove pass

db/models.py Outdated


class User(AbstractUser):
USERNAME_FIELD = "username"

Choose a reason for hiding this comment

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

this is by default the same

@@ -15,13 +16,17 @@ def get_movies(
if actors_ids:
queryset = queryset.filter(actors__id__in=actors_ids)

if title:
return queryset.filter(title__icontains=title)

Choose a reason for hiding this comment

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

Why do you return here???

Suggested change
return queryset.filter(title__icontains=title)
queryset = queryset.filter(title__icontains=title)

for ticket_data in tickets:
row = ticket_data.get("row")
seat = ticket_data.get("seat")
movie_session = MovieSession.objects.get(pk=1)

Choose a reason for hiding this comment

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

Not fixed why pk=1???

date: datetime = None
) -> None:
user, created = User.objects.get_or_create(username=username)
order, created = Order.objects.get_or_create(user=user)

Choose a reason for hiding this comment

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

Suggested change
order, created = Order.objects.get_or_create(user=user)
order = Order.objects.create(user)

One use can have many orders

Comment on lines 36 to 41
if username is not None:
return Order.objects.filter(
user__username=username).order_by("created_at"
)
elif username is None:
return Order.objects.all().order_by("-created_at")

Choose a reason for hiding this comment

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

Suggested change
if username is not None:
return Order.objects.filter(
user__username=username).order_by("created_at"
)
elif username is None:
return Order.objects.all().order_by("-created_at")
orders = Order.objects.all()
if username is not None:
orders = orders.filter(user__username=username)
return orders.order_by("-created_at")

ticket.save()


def get_orders(username: str = None) -> list:

Choose a reason for hiding this comment

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

it returns QuerySet, not list

services/user.py Outdated
user.last_name = last_name
user.save()
return user
except User.DoesNotExist:

Choose a reason for hiding this comment

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

Remove try-except!!!

services/user.py Outdated
Comment on lines 24 to 26
try:
user = User.objects.get(id=user_id)
except User.DoesNotExist:

Choose a reason for hiding this comment

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

Not fixed!

services/user.py Outdated
Comment on lines 24 to 26
try:
user = User.objects.get(id=user_id)
except User.DoesNotExist:

Choose a reason for hiding this comment

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

do it

ticket.save()


def get_orders(username: str = None) -> QuerySet:

Choose a reason for hiding this comment

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

Suggested change
def get_orders(username: str = None) -> QuerySet:
def get_orders(username: str = None) -> QuerySet[Order]:

username: str,
date: datetime = None
) -> None:
user, created = User.objects.get_or_create(username=username)

Choose a reason for hiding this comment

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

Why do you create user? Use just get

for ticket_data in tickets:
row = ticket_data.get("row")
seat = ticket_data.get("seat")
movie_session = MovieSession.objects.filter().first()

Choose a reason for hiding this comment

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

Why do you use .filter(). Also this line is redundant

Choose a reason for hiding this comment

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

this line redundant

ticket, created = Ticket.objects.get_or_create(
row=row,
seat=seat,
movie_session=movie_session,

Choose a reason for hiding this comment

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

Suggested change
movie_session=movie_session,
movie_session_id=and here id of movie session,

Copy link
Author

Choose a reason for hiding this comment

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

поясни, що означає "movie_session_id=and here id of movie session,"

Choose a reason for hiding this comment

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

Suggested change
movie_session=movie_session,
movie_session_id=ticket_data["movie_session"],

db/models.py Outdated

def __str__(self) -> str :
return (
f"{Movie.objects.first()} {self.movie_session.show_time} " # noqa

Choose a reason for hiding this comment

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

Why do you use Movie.objects.first() here?

ticket, created = Ticket.objects.get_or_create(
row=row,
seat=seat,
movie_session=movie_session,

Choose a reason for hiding this comment

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

Suggested change
movie_session=movie_session,
movie_session_id=ticket_data["movie_session"],

for ticket_data in tickets:
row = ticket_data.get("row")
seat = ticket_data.get("seat")
movie_session = MovieSession.objects.filter().first()

Choose a reason for hiding this comment

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

this line redundant

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