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

Resloved #516

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

Conversation

VladyslavShepilov
Copy link

No description provided.

class Migration(migrations.Migration):

dependencies = [
('db', '0001_initial'),

Choose a reason for hiding this comment

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

change ' to ", use Black



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

Choose a reason for hiding this comment

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

use get_movies_sessions here

if show_time:
session.show_time = show_time
if movie_id:
session.movie = Movie.objects.get(id=movie_id)

Choose a reason for hiding this comment

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

use get_movie_by_id here

db/models.py Outdated
class Movie(models.Model):
title = models.CharField(max_length=255)
description = models.TextField()
actors = models.ManyToManyField(Actor, related_name="actors", blank=True)

Choose a reason for hiding this comment

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

wrong related name

Choose a reason for hiding this comment

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

db/models.py Outdated
title = models.CharField(max_length=255)
description = models.TextField()
actors = models.ManyToManyField(Actor, related_name="actors", blank=True)
genres = models.ManyToManyField(Genre, related_name="genres", blank=True)

Choose a reason for hiding this comment

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

same here

from django.db.models import QuerySet


def get_cinema_halls() -> 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_cinema_halls() -> QuerySet:
def get_cinema_halls() -> QuerySet[CinemaHall]:

Comment on lines 36 to 41
if show_time:
session.show_time = show_time
if movie_id:
session.movie = get_movie_by_id(movie_id)
if cinema_hall_id:
session.cinema_hall = get_cinema_halls().get(id=cinema_hall_id)
Copy link

Choose a reason for hiding this comment

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

Suggested change
if show_time:
session.show_time = show_time
if movie_id:
session.movie = get_movie_by_id(movie_id)
if cinema_hall_id:
session.cinema_hall = get_cinema_halls().get(id=cinema_hall_id)
if show_time is not None:
session.show_time = show_time
if movie_id is not None:
session.movie_id = movie_id
if cinema_hall_id is not None:
session.cinema_hall_id = cinema_hall_id

Copy link
Author

@VladyslavShepilov VladyslavShepilov Oct 31, 2023

Choose a reason for hiding this comment

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

It leads to error because we can not assign values directly, he only accepts instance of the classes and I can't find how to pass ids directly. I just can't pass tests with these changes

Comment on lines 31 to 33
show_time: datetime = 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.

better to use Optional in annotation

Copy link
Author

Choose a reason for hiding this comment

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

Ok, added optional None

@VladyslavShepilov
Copy link
Author

FAILED tests/test_main.py::test_movie_session_service_update_movie_session_movie - ValueError: Cannot assign "5": "MovieSession.movie" must be a "Movie" instance.

Comment on lines 10 to 11
cinema_hall = CinemaHall.objects.get(id=cinema_hall_id)
movie = Movie.objects.get(id=movie_id)

Choose a reason for hiding this comment

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

No need to get objects
You can create objects by id: .create(movie_id=movie_id,.....

Comment on lines 18 to 20
if session_date:
return MovieSession.objects.filter(show_time__date=session_date)
return MovieSession.objects.all()

Choose a reason for hiding this comment

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

Suggested change
if session_date:
return MovieSession.objects.filter(show_time__date=session_date)
return MovieSession.objects.all()
movie_sessions = MovieSession.objects.all()
if session_date:
return movie_sessions.filter(show_time__date=session_date)
return movie_sessions

This can help support your code in future

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.

6 participants