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

Solution #950

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# Generated by Django 4.0.2 on 2024-11-12 00:30

from django.conf import settings
import django.contrib.auth.models
import django.contrib.auth.validators
from django.db import migrations, models
import django.db.models.deletion
import django.utils.timezone


class Migration(migrations.Migration):

dependencies = [
('auth', '0012_alter_user_first_name_max_length'),
('db', '0001_initial'),
]

operations = [
migrations.CreateModel(
name='User',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('password', models.CharField(max_length=128, verbose_name='password')),
('last_login', models.DateTimeField(blank=True, null=True, verbose_name='last login')),
('is_superuser', models.BooleanField(default=False, help_text='Designates that this user has all permissions without explicitly assigning them.', verbose_name='superuser status')),
('username', models.CharField(error_messages={'unique': 'A user with that username already exists.'}, help_text='Required. 150 characters or fewer. Letters, digits and @/./+/-/_ only.', max_length=150, unique=True, validators=[django.contrib.auth.validators.UnicodeUsernameValidator()], verbose_name='username')),
('first_name', models.CharField(blank=True, max_length=150, verbose_name='first name')),
('last_name', models.CharField(blank=True, max_length=150, verbose_name='last name')),
('email', models.EmailField(blank=True, max_length=254, verbose_name='email address')),
('is_staff', models.BooleanField(default=False, help_text='Designates whether the user can log into this admin site.', verbose_name='staff status')),
('is_active', models.BooleanField(default=True, help_text='Designates whether this user should be treated as active. Unselect this instead of deleting accounts.', verbose_name='active')),
('date_joined', models.DateTimeField(default=django.utils.timezone.now, verbose_name='date joined')),
('groups', models.ManyToManyField(blank=True, help_text='The groups this user belongs to. A user will get all permissions granted to each of their groups.', related_name='user_set', related_query_name='user', to='auth.Group', verbose_name='groups')),
('user_permissions', models.ManyToManyField(blank=True, help_text='Specific permissions for this user.', related_name='user_set', related_query_name='user', to='auth.Permission', verbose_name='user permissions')),
],
options={
'verbose_name': 'user',
'verbose_name_plural': 'users',
'abstract': False,
},
managers=[
('objects', django.contrib.auth.models.UserManager()),
],
),
Comment on lines +19 to +44

Choose a reason for hiding this comment

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

The User model is being created here, but it seems to duplicate the default Django User model. If you intend to extend the default User model, consider using a custom user model by extending AbstractUser or AbstractBaseUser instead of creating a new model from scratch.

Comment on lines +19 to +44

Choose a reason for hiding this comment

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

Creating a custom User model directly in the migration file is not recommended. Define the model in models.py and then create migrations. This ensures better maintainability and integration with Django's authentication system.

migrations.CreateModel(
name='Order',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('created_at', models.DateTimeField(auto_now_add=True)),
('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='orders', to=settings.AUTH_USER_MODEL)),
Comment on lines +49 to +50

Choose a reason for hiding this comment

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

The Order model references settings.AUTH_USER_MODEL for the user field, which is correct. Ensure that AUTH_USER_MODEL is set to the correct user model in your settings if you are using a custom user model.

Comment on lines +49 to +50

Choose a reason for hiding this comment

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

The user field in the Order model uses on_delete=CASCADE. Ensure this behavior aligns with your application's logic, as deleting a user will also delete all related orders.

],
options={
'ordering': ('-created_at',),
},
),
migrations.AlterField(
model_name='movie',
name='actors',
field=models.ManyToManyField(related_name='movies', to='db.Actor'),
),
migrations.AlterField(
model_name='movie',
name='genres',
field=models.ManyToManyField(related_name='movies', to='db.Genre'),
),
migrations.AlterField(
model_name='movie',
name='title',
field=models.CharField(db_index=True, max_length=255),
),
migrations.AlterField(
model_name='moviesession',
name='cinema_hall',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='movie_sessions', to='db.cinemahall'),
),
migrations.AlterField(
model_name='moviesession',
name='movie',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='movie_sessions', to='db.movie'),
),
migrations.CreateModel(
name='Ticket',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('row', models.IntegerField()),
('seat', models.IntegerField()),
('movie_session', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='db.moviesession')),
('order', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='db.Order')),
],
),
migrations.AddConstraint(
model_name='ticket',
constraint=models.UniqueConstraint(fields=('row', 'seat', 'movie_session'), name='unique_ticket'),
Comment on lines +91 to +93

Choose a reason for hiding this comment

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

The unique constraint on the Ticket model ensures that each combination of row, seat, and movie_session is unique. This is a good practice to prevent duplicate tickets for the same seat in a session.

),
]
20 changes: 20 additions & 0 deletions db/migrations/0003_alter_order_user.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Generated by Django 4.0.2 on 2024-11-12 01:55

from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
('db', '0002_user_order_alter_movie_actors_alter_movie_genres_and_more'),
]

operations = [
migrations.AlterField(
model_name='order',
name='user',
field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='orders', to=settings.AUTH_USER_MODEL),
Comment on lines +17 to +18

Choose a reason for hiding this comment

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

The user field in the Order model is now nullable and can be left blank. This means an Order can exist without being associated with a User. Ensure this change is intentional and that your application logic can handle orders without users.

Comment on lines +17 to +18

Choose a reason for hiding this comment

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

The user field in the Order model is now set to allow null and blank values. Ensure this change aligns with your application's logic, as it allows orders to exist without an associated user.

),
]
68 changes: 67 additions & 1 deletion db/models.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
from django.contrib.auth.models import AbstractUser
from django.db import models
from django.core.exceptions import ValidationError
from django.db.models import UniqueConstraint
from django.conf import settings


class Genre(models.Model):
Expand All @@ -17,7 +21,7 @@ def __str__(self) -> str:


class Movie(models.Model):
title = models.CharField(max_length=255)
title = models.CharField(max_length=255, db_index=True)
description = models.TextField()
actors = models.ManyToManyField(to=Actor, related_name="movies")
genres = models.ManyToManyField(to=Genre, related_name="movies")
Expand Down Expand Up @@ -50,3 +54,65 @@ class MovieSession(models.Model):

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


class Order(models.Model):
created_at = models.DateTimeField(auto_now_add=True)
user = models.ForeignKey(settings.AUTH_USER_MODEL,
on_delete=models.CASCADE,
null=True,
blank=True,

Choose a reason for hiding this comment

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

i think null and blank dont need here

related_name="orders")

def __str__(self) -> str:
return str(self.created_at)

class Meta:
ordering = ("-created_at",)


class Ticket(models.Model):
movie_session = models.ForeignKey(to=MovieSession,
on_delete=models.CASCADE)
order = models.ForeignKey(to=Order,
on_delete=models.CASCADE)
row = models.IntegerField()
seat = models.IntegerField()

class Meta:
constraints = [
UniqueConstraint(
fields=["row", "seat", "movie_session"],
name="unique_ticket"
)
]

def __str__(self) -> str:
return f"{self.movie_session} (row: {self.row}, seat: {self.seat})"

def clean(self) -> None:
if self.row < 1 or self.row > self.movie_session.cinema_hall.rows:
raise ValidationError({
"row": [
f"row number must be in available range: (1, rows): (1, "
f"{self.movie_session.cinema_hall.rows})"
]
})

if (self.seat < 1
or self.seat > self.movie_session.cinema_hall.seats_in_row):
raise ValidationError({
"seat": [
f"seat number must be in "
f"available range: (1, seats_in_row): (1, "
f"{self.movie_session.cinema_hall.seats_in_row})"
]
})
Comment on lines +93 to +110

Choose a reason for hiding this comment

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

The clean method in the Ticket model correctly validates the row and seat fields against the CinemaHall capacity. Ensure that this validation logic aligns with your application's requirements and that it is tested to prevent invalid data entry.


def save(self, *args, **kwargs) -> None:
self.full_clean()
super().save(*args, **kwargs)


class User(AbstractUser):
pass
Comment on lines +117 to +118

Choose a reason for hiding this comment

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

The User model extends AbstractUser but does not add any additional fields or methods. If you plan to customize the user model further, consider adding fields or methods that are specific to your application's needs.

23 changes: 15 additions & 8 deletions services/movie.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from django.db import transaction
from django.db.models import QuerySet

from db.models import Movie
Expand All @@ -6,6 +7,7 @@
def get_movies(
genres_ids: list[int] = None,
actors_ids: list[int] = None,
Comment on lines 8 to 9

Choose a reason for hiding this comment

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

The parameters genres_ids and actors_ids are initialized with None. Consider using an empty list as a default value to avoid potential issues with NoneType operations. However, be cautious as using mutable default arguments can lead to unexpected behavior if not handled properly.

title: str = None,
) -> QuerySet:
queryset = Movie.objects.all()

Expand All @@ -15,6 +17,9 @@ def get_movies(
if actors_ids:
queryset = queryset.filter(actors__id__in=actors_ids)

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

return queryset


Expand All @@ -28,13 +33,15 @@ def create_movie(
genres_ids: list = None,
actors_ids: list = None,
Comment on lines 33 to 34

Choose a reason for hiding this comment

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

Similar to the get_movies function, consider using an empty list as a default value for genres_ids and actors_ids to avoid potential issues with NoneType operations. Ensure proper handling to avoid mutable default argument pitfalls.

Comment on lines 33 to 34

Choose a reason for hiding this comment

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

The type annotations for genres_ids and actors_ids should specify list[int] for consistency and clarity, similar to the get_movies function.

) -> Movie:
movie = Movie.objects.create(
title=movie_title,
description=movie_description,
)
if genres_ids:
movie.genres.set(genres_ids)
if actors_ids:
movie.actors.set(actors_ids)

with transaction.atomic():
movie = Movie.objects.create(
title=movie_title,
description=movie_description,
)
if genres_ids:
movie.genres.set(genres_ids)
if actors_ids:
movie.actors.set(actors_ids)

return movie
14 changes: 12 additions & 2 deletions services/movie_session.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from django.db.models import QuerySet

from db.models import MovieSession
from db.models import MovieSession, Ticket


def create_movie_session(
Expand Down Expand Up @@ -41,4 +41,14 @@ def update_movie_session(


def delete_movie_session_by_id(session_id: int) -> None:
MovieSession.objects.get(id=session_id).delete()
try:
MovieSession.objects.get(id=session_id).delete()
except MovieSession.DoesNotExist:
print(f"Movie session with id {session_id} does not exist.")


def get_taken_seats(movie_session_id: int) -> list:

Choose a reason for hiding this comment

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

There's a typo in the function name get_taken_seats in the previous review. Ensure the function name is correctly used throughout the codebase to avoid confusion.

Choose a reason for hiding this comment

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

Agree

return [
{"row": ticket.row, "seat": ticket.seat}
for ticket in Ticket.objects.filter(movie_session_id=movie_session_id)
]
51 changes: 51 additions & 0 deletions services/order.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
from django.contrib.auth import get_user_model
from django.core.exceptions import ObjectDoesNotExist
from django.db import transaction
from django.utils.datetime_safe import datetime

from db.models import Order, Ticket, MovieSession


def create_order(tickets: list, username: str, date: datetime = None) -> Order:

with transaction.atomic():
try:
user = get_user_model().objects.get(username=username)
except ObjectDoesNotExist:

Choose a reason for hiding this comment

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

Consider using User.DoesNotExist instead of ObjectDoesNotExist for clarity and specificity in exception handling.

Choose a reason for hiding this comment

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

Agree

raise ValueError(
f"User with username '{username}' does not exist."
)

order = Order.objects.create(user=user)
if date:
order.created_at = date
order.save()

create_tickets = [
Ticket(
row=ticket.get("row"),
seat=ticket.get("seat"),
movie_session=MovieSession.objects.get(
id=ticket.get("movie_session")
),
order=order
)
for ticket in tickets
]

Ticket.objects.bulk_create(create_tickets)

return order


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.

The return type of get_orders should be annotated as QuerySet instead of list for accuracy.

Choose a reason for hiding this comment

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

Agree

if username:
try:
user = get_user_model().objects.get(username=username)
except ObjectDoesNotExist:

Choose a reason for hiding this comment

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

Consider using User.DoesNotExist instead of ObjectDoesNotExist for clarity and specificity in exception handling.

Choose a reason for hiding this comment

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

Agree

raise ValueError(
f"User with username '{username}' does not exist."
)
return Order.objects.filter(user=user)
else:
return Order.objects.all()
53 changes: 53 additions & 0 deletions services/user.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
from django.contrib.auth import get_user_model
from django.core.exceptions import ObjectDoesNotExist

User = get_user_model()


def create_user(username: str,
password: str,
email: str = None,
first_name: str = None,
last_name: str = None) -> User:
first_name = first_name or ""
last_name = last_name or ""
user = User.objects.create_user(
username=username,
password=password,
email=email,
first_name=first_name,
last_name=last_name
)
return user


def get_user(user_id: int) -> User:
try:
return User.objects.get(pk=user_id)
except ObjectDoesNotExist:

Choose a reason for hiding this comment

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

Consider using User.DoesNotExist instead of ObjectDoesNotExist for clarity and specificity in exception handling.

Choose a reason for hiding this comment

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

Agree

raise ValueError(f"User with ID {user_id} does not exist.")


def update_user(user_id: int,
username: str = None,
password: str = None,
email: str = None,
first_name: str = None,
last_name: str = None) -> User:
try:
user = User.objects.get(pk=user_id)
if username:
Comment on lines +38 to +39

Choose a reason for hiding this comment

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

Use get_user method here

user.username = username
if password:
user.set_password(password)
if email:
user.email = email
if first_name:
user.first_name = first_name
if last_name:
user.last_name = last_name

user.save()
return user
except ObjectDoesNotExist:

Choose a reason for hiding this comment

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

Consider using User.DoesNotExist instead of ObjectDoesNotExist for clarity and specificity in exception handling.

Choose a reason for hiding this comment

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

Agree

raise ValueError(f"User with ID {user_id} not found. Update failed.")
4 changes: 4 additions & 0 deletions settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,8 @@

INSTALLED_APPS = [
"db",
"django.contrib.auth",
"django.contrib.contenttypes",
Comment on lines 27 to +29

Choose a reason for hiding this comment

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

db should be the last one in the list

]

AUTH_USER_MODEL = "db.User"
Loading