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 solution #839

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

Implemented solution #839

wants to merge 3 commits into from

Conversation

dimak20
Copy link

@dimak20 dimak20 commented Aug 6, 2024

No description provided.

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

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 because it defines correct settings(as there may be a folder named settings)

db/models.py Outdated
@@ -17,14 +23,19 @@ def __str__(self) -> str:


class Movie(models.Model):
title = models.CharField(max_length=255)
title = models.CharField(max_length=255, db_index=True)
Copy link

Choose a reason for hiding this comment

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

either db_index = True or indexes should be used. indexes are often used for complex indexes



class Order(models.Model):
created_at = models.DateTimeField(default=datetime.datetime.now)
Copy link

Choose a reason for hiding this comment

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

use auto_now_add instead

Copy link
Author

Choose a reason for hiding this comment

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

I tried but It did not work due to mocking datetime. And It would be too strange to override save methods just to pass all the tests

db/models.py Outdated
class Order(models.Model):
created_at = models.DateTimeField(default=datetime.datetime.now)
user = models.ForeignKey(
settings.AUTH_USER_MODEL, on_delete=models.CASCADE,
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

@@ -6,6 +7,7 @@
def get_movies(
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.

str or None

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

Ticket.objects.bulk_create(raw_tickets)


def get_orders(username: str = None) -> QuerySet:
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
def create_user(
username: str,
password: str,
email: 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

services/user.py Outdated
Comment on lines 28 to 32
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.

str or None

@dimak20 dimak20 requested a review from MNDonut August 30, 2024 14:49
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.

2 participants