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

add registration #46

Merged
merged 10 commits into from
Dec 16, 2023
Merged

add registration #46

merged 10 commits into from
Dec 16, 2023

Conversation

Aleksey-Krd
Copy link
Contributor

@Aleksey-Krd Aleksey-Krd commented Dec 8, 2023

Description

Если нужно что-то дополнить по внесённым изменениям.

Type of change

Пожалуйста, удалите варианты, которые не относятся к ПР-у.

  • Documentation (опечатки, примеры кода или любое обновление документации)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Опишите, пожалуйста, тесты, которые вы провели для проверки ваших изменений. Предоставьте инструкции, чтобы мы могли воспроизвести их. Также укажите все необходимые детали конфигурации тестов.

Checklist:

  • Мой код соответствует code-style данного проекта
  • Я провел самоанализ собственного кода
  • Я внес соответствующие изменения в документацию

@@ -54,7 +57,7 @@
DATABASES = {
'default': {
'ENGINE': 'django.db.backends.sqlite3',
'NAME': BASE_DIR / 'db.sqlite3',
'NAME': os.path.join(BASE_DIR, 'db.sqlite3'),
Copy link
Member

Choose a reason for hiding this comment

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

А что у тебя тут не сработало?..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

не помню, ругалось вроде, я заменил, хотя вроде не должно

Copy link
Member

Choose a reason for hiding this comment

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

Давай попробуем вернуть и еще раз затестим. Лучше уходить от этих лишний join


BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))

SECRET_KEY = get_random_secret_key()
Copy link
Member

Choose a reason for hiding this comment

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

А вот тут непонятно. Зачем?)

Copy link
Member

Choose a reason for hiding this comment

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

Меня больше волнует, что при каждом запуске проекта разве не будет новый генериться?

Copy link
Contributor Author

@Aleksey-Krd Aleksey-Krd Dec 10, 2023

Choose a reason for hiding this comment

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

будет, но меня ревьюер учил делать так), не нужно значит уберём, больше уже привычка

Copy link
Member

Choose a reason for hiding this comment

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

Ну.. Обычно эту функцию используют для генерации секретного ключа (соли). А потом уже прячут в env.
А так опасаюсь, что могут возникнуть проблемы при перезапуске приложения и генерации нового ключа на проде. Вот статейка есть как используют https://codinggear.blog/django-generate-secret-key/#:~:text=Django%20provides%20us%20a%20function,it%20into%20the%20settings.py%20file

@@ -5,4 +5,5 @@
path('admin/', admin.site.urls),
path('', include('main.urls', namespace='main')),
path('auth/', include('users.urls', namespace='users')),
path('auth/', include('django.contrib.auth.urls')),
Copy link
Member

Choose a reason for hiding this comment

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

две ручки с auth??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

удалил уже, заметил уже после комита

from django.urls import path

from . import views
Copy link
Member

Choose a reason for hiding this comment

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

Лучше абсолютные импорты

@KonstantinRaikhert
Copy link
Member

@Aleksey-Krd Авторизация работает и это здорово! Но совсем не так, как мы предполагаем))
Вот на скрине у нас есть форма. Она вылезает нам при переходе на наше приложение. Т.е. нам нельзя показывать данные, пока мы не авторизуемся. Поэтому давай сделаем так же? Ну можно пока твои формы просто вынести отдельно без рисования. Но если ты тут продвинулся, то давай сразу как по макету. Даже если они немного "кривые"
2023-12-11_11-23

@Aleksey-Krd
Copy link
Contributor Author

Aleksey-Krd commented Dec 11, 2023

Я уже сделал, что при переходе на сайт нужна авторизация, идёт редирект на логин, декоратор же есть, просто я ещё форму восстановления пароля не сверстал, вчера не смог попасть за комп) итак же убрал возможность регистрации, она не требуется, администратор добавляет данные

@KonstantinRaikhert
Copy link
Member

@Aleksey-Krd Ну пока при переходе на корневой адрес пока вот так..
2023-12-11_15-06

@KonstantinRaikhert
Copy link
Member

@Aleksey-Krd Получилось?)

@Aleksey-Krd
Copy link
Contributor Author

в процессе, только сел час назад, никак у меня из-за работы не получается толком сесть и сделать

@Aleksey-Krd Aleksey-Krd reopened this Dec 12, 2023
@Aleksey-Krd
Copy link
Contributor Author

Aleksey-Krd commented Dec 12, 2023

@KonstantinRaikhert мне не нужно делать пул реквест правильно? только запушить? а то тут у меня опыта как бы вообще нет, я запушил в общем изменения, посмотри сделал шаблоны, они пока пустышки, удалил ненужное

@KonstantinRaikhert
Copy link
Member

@Aleksey-Krd Нет, если у тебя уже открыт ПР с этой веткой то коммиты отображаются автоматически. Единственное что если хочешь, чтобы посмотрели, нужно запросить request review

Copy link
Member

@KonstantinRaikhert KonstantinRaikhert left a comment

Choose a reason for hiding this comment

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

В целом давай уже это быстренько поправим и смержим, потом будем потихоньку допиливать шаблоны и вьюхи. А то как то засели на таске...

from pathlib import Path

import environ
Copy link
Member

Choose a reason for hiding this comment

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

Ты вроде как поставил его, но не внёс в зависимости, из за этого не запускается проект..
Давай вообще сейчас его уберем, другой разработчик полностью добавит его. Сейчас secret_key неважно какой тут будет хоть пустая строка.

STATIC_ROOT = os.path.join(BASE_DIR, 'static')
# STATICFILES_DIRS = [os.path.join(BASE_DIR, 'static')]

STATICFILES_DIRS = BASE_DIR / 'static',
Copy link
Member

Choose a reason for hiding this comment

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

Нужно разобраться со статикой!)
Во первых в этом исполнении он требует STATIC_ROOT
А во вторых ты собственно свою статику забыл добавить в ПР и поэтому выглядит вот так
2023-12-14_12-49
2023-12-14_12-49_1

Copy link
Contributor Author

@Aleksey-Krd Aleksey-Krd left a comment

Choose a reason for hiding this comment

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

@KonstantinRaikhert убрал статику из игнора, посмотри

Copy link
Member

@KonstantinRaikhert KonstantinRaikhert left a comment

Choose a reason for hiding this comment

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

мержим

@KonstantinRaikhert KonstantinRaikhert merged commit d27e9a6 into dev Dec 16, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants