prrromanssss / intensiveyandex-backend Goto Github PK
View Code? Open in Web Editor NEWThis project is a repository for Django intensive homework from the Yandex Academy
This project is a repository for Django intensive homework from the Yandex Academy
with
и классы bootstrap
, так и через использование django-active-link
(для интереса посмотри) — приму любой вариант.Плюсы:
Карточки товаров хардкод, но пока допущу, в базу чуть позже полезть должны.
И (совсем дополнительно) поискать, как брать актуальный для пользователя год, чтобы помещать его в копирайт
.only('name', 'text', 'category_id', 'category__name')
А нам 'category_id' правда надо?
ItemManager в models.py:
return query_set # noqa
???
https://github.com/Prrromanssss/LyceumYandex_django/blob/59b28742c192137575cc2b37683eeaa4c61b7ab5/lyceum/catalog/tests.py#L4
Что тесты что-то пропустили, указано в issue к urls.
Положительные, отрицательные, ноль/нули, строки с ведущим нулём/нулями, смешанные строки (начинающиеся с числа, с буквы, со спецсимвола, заканчивающиеся ими же и содержащие их посередине в разных вариациях)...
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/env_example.txt#L1
Пример примером, но пробелы вокруг = не нужны. Как не нужны и кавычки, там все значения строковые.
И, если называешь файл что-то-там-example, а не что-то-там-var-datatype-instruction, лучше сразу дать этот example, а не загадочную всё же инструкцию по типам данных. Представь, что сам впервые видишь проект и читаешь к нему инструкцию, всё должно быть максимально просто и логично. Например, SECRET_KEY=YOUR_SECRET_KEY_123 и DEBUG=True
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/lyceum/catalog/models.py#L25
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/lyceum/catalog/models.py#L45
Ты точно валидаторы MinLenValidator собирался использовать?..
Это не говоря об условиях (например, условие про 2 символа я в ТЗ не нашёл ☝️)
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/lyceum/catalog/tests.py#L19
Почти оптимально. Кому-то не понравится, как списки хранятся или ещё что, но в целом очень гуд.
В зависимости от соотношения оптимизация/информативность можно остановиться на этом или объединить этот абзац с последующим, добавив перед этим один for и скорректировав то, что передается в скобках .subTest()
Но меня и так устраивает :)
https://github.com/Prrromanssss/LyceumYandex_django/blob/59b28742c192137575cc2b37683eeaa4c61b7ab5/lyceum/homepage/urls.py#L2
Почти то же самое, что с about/urls.py. Но надо только 1 пустую строку добавить
Смотрю в качестве решения основной части (расширение через 1:1, оно же o2o) - в этом плане ок
Разве что можно дообезопаситься в части birthday дополнительным blank=True
А в реальном проекте по необходимости ещё валидация была бы навешана (но здесь это не нужно)
https://github.com/Prrromanssss/LyceumYandex_django/blob/59b28742c192137575cc2b37683eeaa4c61b7ab5/lyceum/catalog/tests.py#L5
Почему test_negative_endpoins? Тестирует только отрицательные значения? Нет, тестирует на неподходящие значения. Ищем в словаре нужное слово, переименовываем.
И почему это противопоставляется test_static_endpoints? Мы разве противопоставляем тестирование статических урлов динамически генерируемым? Вроде, нет.
И раз мы так разбиваем, что везде сравниваем ответ с одним и тем же статусом (в одной функции - 200, в другой - 400; а значит, и проверка типовая-аналогичная), посмотри в сторону subTest.
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/lyceum/catalog/tests.py#L36
Название тестов не отражает их содержание.
Где нужна валидация, использовать менеджер контекста (with self.assertRaises(ValidationError)
).
Здесь тоже можно в subTest, но не обязательно, и так задач много.
Проверяем добавление одного из требуемых слов, второго, можно обоих сразу (но не обязательно), и фразы без этих слов.
А что будет, если в описании будет строка типа "Это роскошно!
"?
Протестируй, вдруг выявятся ещё какие-то недостатки
При создании поля превью в Item требуются название, категория, тег... ещё и слова обязательные требует... мы точно превью создаем, а не экземпляр Item? o_O
Ещё и ошибки возникают, если создать с тем же превью, например
Что-то работает вообще совсем не так, как надо. Попробуй-ка сам насоздавать разных вариантов
При создании Item мы должны иметь возможно задать ему главную картинку + галерею из картинок.
И не надо заставлять пользователя/админа перед этим отдельно заносить картинку в базу. Подумай над отдельными моделями для этого
Как думаешь, может стоит создать отдельную модельку для изображения, от него унаследовать модель главного изображения и модели изображения для галереи?
Вдруг это даст какие-то новые возможности помимо того, что единообразнее работа с изображениями будет?
В итоге мы должны удобно добавлять/изменять/удалять изображения прямо из карточки создания/редактирования товара
Кстати, Preview и Gallery логичнее будет унаследовать от общего предка, не находишь?
В админке странный порядок полей, при создании тега или категории вторичное поле слага идёт до названия, например, что по логике странно
https://github.com/Prrromanssss/LyceumYandex_django/blob/81caed7d7656a5ffc98857027fc0e4a456e86f2a/lyceum/core/models.py#L21
Не исправил лишний перенос, например
https://github.com/Prrromanssss/LyceumYandex_django/blob/59b28742c192137575cc2b37683eeaa4c61b7ab5/lyceum/about/urls.py#L2
Достаточно со 2-й строки переместить from . import views на 3-ю (сверху и снизу будет по 1 пустой строке), чтобы недостаток был устранён.
Почему? Читаем PEP8 (и вообще гайды по стилю читаем).
Между Tag и Item связь m:n, верно? От catalog_tag к вспомогательной таблице catlog_item_tags вилка не в ту сторону.
На варианты указания типов данных (char/varchar, int/bigint/smallint,..) и ограничения типов про конкретным БД сейчас не обращаю внимания, важнее смысл. Остальное подтянется, к тому же зависит от типа БД.
Уточни этот момент и приму, здесь спокойно 3 балла будет, только сделай быстро
https://github.com/Prrromanssss/LyceumYandex_django/blob/59b28742c192137575cc2b37683eeaa4c61b7ab5/lyceum/catalog/urls.py#L2
То же самое, что с about/urls.py.
Буду рад, — кто ещё так не делает, — если кроме обязательных миграций и фикстур будете прикладывать пример БД (как во 2-й лекции упоминалось), это немного сокращает время проверки.
А в ридми указать, что приложена тестовая БД чисто для знакомства с её структурой и с админкой.
Ну и данные учётки тестового админа не забыть указать. Спасибо)
Я добавил субтесты и перебрал url в цикле, посмотри, пожалуйста, так лучше выглядит или нет?
Файлик catalog.tests.py
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/lyceum/catalog/models.py#L47
...
...
...
Ну, веса для сортировки обычно используются. Но в задании этого не вижу, поэтому пока исходим из текущей информации, укажи просто "Максимум 32767"
Категорию в карточке товара на странице списка указывать не надо, внимательнее смотрим ТЗ
Поэтому, поскольку используем одну и ту же карточку, надо добавить в неё условия. Как минимум, это условие "если на странице списка, не показывать в карточке категорию"
А ещё вполне стоит использовать эту же карточку и на странице конкретного товара, чтобы не плодить код, - просто поставив условием отображения в ней полного описания то, что мы на странице отдельного товара
Очень плохо, что в карточках нет перехода на конкретную карточку товара
Ладно, мы с тобой, можем ввести число в адресную строку, а представь человека за 60, he/she-то что делать будет
Кстати, попробуй перейти на карточку товара. Всё ли работает? ;)
И жалко, что в карточках нет изображений, согласись. Может, уже стоит добавить? (хоть этого пока и нет в задании)
Не обязательно, но можно ещё продублировать в карточке товара ссылку на возврат в список, будет не страшно
А вот такие вещи упрощать надо:
{% if forloop.last %}
{{ tag.name }}.
{% else %}
{{ tag.name }},
{% endif %}
зачем повторять {{ tag.name }}
, если вопрос только в знаке?
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/lyceum/catalog/models.py#L21
Такое же значение, что и для category? Может, вынести в default_related_name в Meta?
https://docs.djangoproject.com/en/3.2/ref/models/options/#default-related-name
Можно и в другие модельки добавить
https://github.com/Prrromanssss/LyceumYandex_django/blob/2f4be47126cfd337be968f9560079a7a00f94f04/lyceum/templates/about/index.html
Проверь все генерируемые страницы, возможно какая-то даст сбой. Например, about
.
Подскажу один из вариантов поиска ошибок:
Запускаешь сервер Django, открываешь в браузере для каждой из сгенерированных основных страниц исходный код (обычно Ctrl+U
).
Валидируешь здесь или здесь, например.
Вывалится ряд ошибок, которые надо исправить. Для каждого недостатка в коде будет указано, где он встретился, приоритет, что стоит сделать и в чём может быть проблема, если всё оставить, как есть.
Другой вариант — использование специальных аддонов.
Дополнительно:
Мне бы хватило банального Lorem ipsum, а так какой-нибудь lindeal (или у кого они это спёрли) подадут на авторские, когда репо откроешь для портфолио.
Якорь на Марка Цукерберга ведёт в никуда, фу так делать)
Странно, что в сгенеренном коде <!DOCTYPE html>
идёт только 3-й строчкой
Так-то мы не фронтендеры и не про дизайн особо, но это то, что прям в глаза бросается
https://github.com/Prrromanssss/LyceumYandex_django/blob/59b28742c192137575cc2b37683eeaa4c61b7ab5/lyceum/about/urls.py
https://github.com/Prrromanssss/LyceumYandex_django/blob/59b28742c192137575cc2b37683eeaa4c61b7ab5/lyceum/catalog/urls.py
https://github.com/Prrromanssss/LyceumYandex_django/blob/59b28742c192137575cc2b37683eeaa4c61b7ab5/lyceum/homepage/urls.py
Посмотри, что можно улучшить в urls, используя name и app_name (для пространств имен), и прочитай, почему (https://docs.djangoproject.com/en/4.1/topics/http/urls/#id7).
По вёрстке собо не смотрю, всё же у нас бэк, а не фронт. Но даже в прототипировании стоит быть аккуратнее
На странице about
что-то как-то всё липнет к краю окна бразуера. Думаю, как-то container
надо правильно использовать
Также мелкий момент по оформлению, но уже не темплейтов: элемент словаря контекста где-то заканчивается запятой, где-то нет, выбери какой-то один стиль
Дополнительно по отображению
Не помешает текст-заглушка, когда товаров для вывода нет
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/lyceum/core/admin.py#L1
Приберись в импортах. Лишнее убрать или на время разработки F401 можно в ignore конфига добавить (там указал). Отсортировать импорты по строкам, с нужной отбивкой пустыми строками, и по порядку в рамках строк.
Про pip install flake8 pep8-naming flake8-broken-line flake8-return flake8-isort
для самостоятельного тестирования, чтобы мне к этому не возвращаться, я уже говорил
https://github.com/Prrromanssss/LyceumYandex_django/blob/81caed7d7656a5ffc98857027fc0e4a456e86f2a/lyceum/core/models.py#L12
Почитай про порядок всего внутри модельки
В частности это:
The order of model inner classes and standard methods should be as follows (noting that these are not all required):
All database fields
Custom manager attributes
class Meta
def __str__()
def save()
def get_absolute_url()
Any custom methods
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/setup.cfg#L9
Зачем сейчас per-file-ignores? Любое дополнительное исключение должно быть обоснованно. Сейчас причин нет:
Там скорее в ignore можно для удобства добавлять F401 на время разработки, т.к. джанго любит создавать файлы уже с заготовками импортов
def short_text(self):
return f'{self.text[:10]}...'
Отзыв из 1 буквы - всё равно будет многоточие после неё
(А ещё 10 букв на практике было бы маловато)
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/lyceum/core/models.py#L4
CommonFieldsNameIsPublished - не очень воспринимается, когда упоминаешь при наследовании.
Во-первых, это Model, а не Fields, и название должно говорить об этом. Во-вторых, это некая базовая модель, ну или абстрактная.
Поэтому в твоем случае даже что-то типа IsPublishedNamedBaseModel будет логичнее.
Но здесь стоит подумать ещё вот о чём. Вроде бы у нас есть ТЗ, но там не указано про уникальность name.
Но логически же имена тегов и категорий должны быть уникальны, разве нет? А вот с Item - вопрос, в реальности названия могут совпадать.
Поэтому подумай на свой выбор, как лучше с этим поступить (выносить или переопределять, в каком случае дублирование кода будет более избыточным).
Начнем с того, что они достаточно разрослись, чтобы их разбить на отдельные модули
Создаём вместо такого растолстевшего tests.py
папку tests
в соответствующем приложении и там располагаем всякие test_models.py
, tests_urls.py
, tests_views.py
и подобные им (почитай, какие имена файлов отлавливает Django unittest, чтобы в них искать тесты для запуска)
В этом случае чётко разделяем, где тестируем доступность самих урлов, где вывод вьюх, где особенности моделей (напр., валидацию)
Где-то ещё теперь в старые места можно вписать адресацию с reverse
.
~
Кстати, можешь обратить внимание, что где-то что-то повторяет. Например, этот кусок:
response = Client().get(reverse('homepage:home'))
Но здесь есть разные подходы, иногда такое дублирование может быть и оправданным. В данном случае, например, можно оставить, придираться не буду
А разделенные одиночные subTest ну как-то не оч
https://github.com/Prrromanssss/LyceumYandex_django/blob/81caed7d7656a5ffc98857027fc0e4a456e86f2a/lyceum/core/models.py#L10
Посмотри, может где-то достаточно отдавать self.name
Для единообразия можно было бы снова задекорировать класс в админке users, немного поменяв последовательность блоков кода
Но это некритично
В users.admin сейчас две строчки импортов из django.contrib.auth.admin - сразу вопрос: почему отдельно?
Но сначала почитай, как соотносятся User из from django.contrib.auth.admin и из from django.contrib.auth.models
Может, прослойки из ...auth.admin можно вообще избежать?
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/lyceum/core/models.py#L9
Можно уменьшить количество кода, если указывать verbose_name на 1-м месте, как предлагает сама официальная документация (это не подойдет только для полей ForeignKey, ManyToManyField и OneToOneField). И это не надо считать неединообразным подходом. Лучше единообразить порядок полей в моделях :)
И там же написано про конвенцию наименования verbose_name.
Заодно по другим местам посмотри на help_text, в нем первая буква автоматически не капитализируется.
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/lyceum/catalog/models.py#L24
Что-то не то с отступами при переносе строки
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/lyceum/core/models.py#L22
Можно не придумывать замену слову, "слаг" вполне себе устоявшийся термин, пусть и узкоспециализированный. Если режет глаз, можно оставить английский вариант.
Про правила написания verbose_name упомняул в другом issue.
https://github.com/Prrromanssss/LyceumYandex_django/blob/59b28742c192137575cc2b37683eeaa4c61b7ab5/lyceum/catalog/urls.py#L7
https://www.regular-expressions.info/named.html
Особенно пригодится при усложнении адреса
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/lyceum/catalog/models.py#L18
А надо ли такие описания? Атрибут help_text используется не только для отображения текста в админке, так и может быть использован для дальнейшего вывода на страницу через ModelForm. А если там это будет реализовано через другие контролы (например, чекбоксы)? Пока достаточно будет чего-то типа "Выберите теги". Ну и в других всех местах уточни, где надо.
https://github.com/Prrromanssss/LyceumYandex_django/blob/eb95a342ee46a609a494beab07b7b5e65d1a3dbf/lyceum/catalog/admin.py#L3
Про порядок импортов вроде где-то упомянул, но и здесь напишу.
Заодно здесь ж скажу, что самому будет быстрее видеть общую картину, если развернутые описания @admin.register() будут расположены ниже кратких. Это уже, как нам преподы говорили, вкусовщина, но и факт.
https://github.com/Prrromanssss/LyceumYandex_django/blob/59b28742c192137575cc2b37683eeaa4c61b7ab5/.github/workflows/python-package.yml#L4
Два .yml нам сейчас не нужны. Тем более, что они друг друга фактически дублируют. Просто перенеси Run Tests в ранее созданный workflow, этого будет достаточно.
https://github.com/Prrromanssss/LyceumYandex_django/blob/1307e9f8e5ff1d463ba59a3ad783db5ccf7fd1b8/lyceum/catalog/tests.py#L54
Попробуй в test_without_needed_words() указать одно норм значение, а в test_with_needed_words() - одно не норм. Помотри, что выдадут тесты.
У меня получается, что сразу вывалилось несколько failures, а так не должно быть (всего одно вхождение должно было посыпаться и сказать, какое).
https://github.com/Prrromanssss/LyceumYandex_django/blob/59b28742c192137575cc2b37683eeaa4c61b7ab5/.github/workflows/django.yml#L30
Загляни ручками в GitHub Actions -> Django CI > Run Tests и подумай, почему тесты не проходят:
Ran 0 tests in 0.000s
Должны запускаться и проходить. Вспоминаем команды для работы с директориями в консоли.
Карточка товара используется более одного раза, но относится к каталогу (при этом незначительную вариативность в ней можно задать условными конструкциями).
Скажу своё субъективное мнение, а дальше смотри сам. Здесь случай мелкий, но я всё равно бы руководствовался логикой архитектуры отделимого приложения.
Если главная хочет использовать это приложение - это её проблемы, пускай идёт в него. Это не особо повод поднимать инклюд карточки до уровня проекта.
Поэтому лично мой вариант - инклюд на уровне приложения (templates/catalog/includes/card.html
).
В другом случае я бы подумал об обоснованности того или иного решения с учётом сути проекта.
https://github.com/Prrromanssss/LyceumYandex_django/blob/59b28742c192137575cc2b37683eeaa4c61b7ab5/lyceum/catalog/urls.py#L7
Что-то немного не то с регуляркой (и я такое уже видел).
/catalog/01 (или /catalog/001) пропускает наряду с /catalog/1, но при этом не пропускает что-то типа /catalog/010, хотя пропускает /catalog/10 :)
Вообще, так-то элементы с ведущими нулями пропускать не оч (в частности, в данном случае). Убираем, чтобы не пропускало. Опять же, и задачу это немного упростит ;)
Вынести name у Tag и Category в абстрактную модель
Во-первых, импорты
Дважды django.contrib.auth.forms - можно объединить, и as этому не помеха
Только проверь, чтобы линтеры не ругались
Во-вторых, по формам
Сейчас перемудрил, можно упростить. Но прежде всего пока будем ориентироваться, чтобы работало
Кстати, загляни сюда, возможно позаимствуешь что-то полезное:
https://github.com/Capwell/extend_user_model_django
Конкретно если по 1:1, это здесь:
https://github.com/Capwell/extend_user_model_django/tree/main/one_to_one/users
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.