update user model and registration url - #1
Conversation
| text = models.TextField() | ||
| created_at = models.DateTimeField(auto_now_add=True) | ||
| approved = models.BooleanField(default=False) | ||
| user = models.ForeignKey(User, on_delete=models.CASCADE, default=1) |
There was a problem hiding this comment.
this field should be required and there should be no default here. I can't imagine a situation how a post could be created without a user.
| @@ -0,0 +1,3 @@ | |||
| from django.contrib import admin | |||
|
|
|||
| # Register your models here. | |||
| from django.db import models | ||
| from django.core import validators | ||
| from django.contrib.auth.models import AbstractBaseUser, UserManager | ||
| # from django.contrib.auth.models import PermissionsMixin |
| validators=[validators.validate_email], | ||
| unique=True, | ||
| blank=False | ||
| ) |
| objects = UserManager() | ||
|
|
||
| def __str__(self): | ||
| return self.username |
There was a problem hiding this comment.
consider adding more unique fields to identify user. Are you sure this works? I guess it should be smth like f"{self.username}"
|
|
||
| class Meta: | ||
| model = User | ||
| fields = ('email', 'username', 'password',) |
There was a problem hiding this comment.
why do you need comma at the end in a tuple with more than 1 item?
| @@ -0,0 +1,3 @@ | |||
| from django.test import TestCase | |||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.AddField( |
There was a problem hiding this comment.
consider merging this to the initial migration
| @@ -0,0 +1,32 @@ | |||
| # Generated by Django 3.1.6 on 2021-06-01 22:41 | |||
|
|
|||
There was a problem hiding this comment.
why not have all migrations in one module?
| serializer.save() | ||
|
|
||
| return Response( | ||
| status=status.HTTP_201_CREATED, |
There was a problem hiding this comment.
consider responding with user_id and maybe even all her data
| @@ -0,0 +1,8 @@ | |||
| FROM python:3.7-stretch | |||
| @@ -0,0 +1,8 @@ | |||
| FROM python:3.7-stretch | |||
| RUN apt-get update | |||
| RUN apt-get upgrade -y | |||
There was a problem hiding this comment.
It's better not to upgrade distribution
| @@ -0,0 +1,8 @@ | |||
| FROM python:3.7-stretch | |||
| RUN apt-get update | |||
There was a problem hiding this comment.
- add
--yesflag - Remove cache with
rm -rf /var/lib/apt/lists/* - Why do you need update if there is no installation here.
| RUN apt-get upgrade -y | ||
| COPY ./ / | ||
| WORKDIR ./ | ||
| RUN pip3 install --no-cache-dir -r requirements.txt && \ |
| RUN apt-get update | ||
| RUN apt-get upgrade -y | ||
| COPY ./ / | ||
| WORKDIR ./ |
| app: | ||
| container_name: app | ||
| build: . | ||
| command: bash -c "python manage.py makemigrations && python manage.py migrate && python manage.py runserver 0.0.0.0:8000" |
There was a problem hiding this comment.
If you have more than 1 command it's better to have them in entrypoint.sh file with appropriate linting. It would be easier to maintain and read
| 'USER': 'user', | ||
| 'PASSWORD': 'password', | ||
| 'HOST': '127.0.0.1', | ||
| 'HOST': 'db', |
|
|
||
|
|
||
| class User(AbstractBaseUser): | ||
|
|
There was a problem hiding this comment.
no need in empty line here. Consider adding linting and type checking configs in setup.cfg
| FROM python:3.7-stretch | ||
| RUN apt-get update | ||
| RUN apt-get upgrade -y | ||
| COPY ./ / |
There was a problem hiding this comment.
Consider adding
ENV DEBIAN_FRONTEND=noninteractive
# Keeps Python from generating .pyc files in the container
ENV PYTHONDONTWRITEBYTECODE 1
# Prevents Python from buffering stdout and stderr
ENV PYTHONUNBUFFERED 1
| WORKDIR ./ | ||
| RUN pip3 install --no-cache-dir -r requirements.txt && \ | ||
| rm -v requirements.txt | ||
| EXPOSE 8000 |
There was a problem hiding this comment.
Consider adding:
COPY scripts/entrypoint.sh /app/scripts/entrypoint.sh
ENTRYPOINT ["./scripts/entrypoint.sh"]2 participants
No description provided.