From 2121ee72a18f79b0825392c059b165e95416f5d7 Mon Sep 17 00:00:00 2001 From: dmitriy lyakhovitskiy Date: Wed, 29 Mar 2023 20:07:20 +0300 Subject: [PATCH] review --- yatube/about/urls.py | 2 +- yatube/about/views.py | 2 +- yatube/core/context_processors/year.py | 3 ++ yatube/core/tests.py | 2 +- yatube/core/views.py | 7 ++- yatube/posts/admin.py | 4 +- .../migrations/0002_auto_20230228_1135.py | 2 + yatube/posts/models.py | 5 ++- yatube/posts/urls.py | 3 ++ yatube/posts/views.py | 44 +++++++++++-------- yatube/templates/about/author.html | 3 +- yatube/templates/about/tech.html | 1 + yatube/templates/base.html | 8 ++-- yatube/templates/includes/footer.html | 2 +- yatube/templates/includes/post.html | 2 +- yatube/templates/posts/create_post.html | 2 +- yatube/templates/posts/follow.html | 2 +- yatube/templates/posts/index.html | 2 +- yatube/templates/posts/profile.html | 4 +- yatube/users/admin.py | 1 + yatube/users/forms.py | 2 +- yatube/users/models.py | 1 + yatube/users/tests.py | 1 + yatube/users/urls.py | 1 + yatube/users/views.py | 1 + yatube/yatube/settings.py | 20 ++++++++- 26 files changed, 88 insertions(+), 39 deletions(-) diff --git a/yatube/about/urls.py b/yatube/about/urls.py index 235ff17..8388e73 100644 --- a/yatube/about/urls.py +++ b/yatube/about/urls.py @@ -1,5 +1,5 @@ from django.urls import path -from . import views +from . import views # импорты не по pep8 app_name = 'about' diff --git a/yatube/about/views.py b/yatube/about/views.py index 3cd09a7..c997456 100644 --- a/yatube/about/views.py +++ b/yatube/about/views.py @@ -1,6 +1,6 @@ from django.views.generic.base import TemplateView - +# не должно оставаться учебных комментариев. засоряется код class AboutAuthorView(TemplateView): # В переменной template_name обязательно указывается имя шаблона, # на основе которого будет создана возвращаемая страница diff --git a/yatube/core/context_processors/year.py b/yatube/core/context_processors/year.py index 9eee68c..50b549c 100644 --- a/yatube/core/context_processors/year.py +++ b/yatube/core/context_processors/year.py @@ -1,3 +1,6 @@ +# Для времени в джанге используй `django.utils.timezone`, тогда будет учитываться часовой пояс. Время и разница в часовых поясах это извечная проблема приложений, работающих с пользователями в различных часовых поясах. На первый взгляд подобная мелочь может вызвать довольно много ошибок. +# https://docs.djangoproject.com/en/4.1/ref/utils/#module-django.utils.timezone +# https://stackoverflow.com/questions/10783864/django-1-4-timezone-now-vs-datetime-datetime-now from datetime import datetime diff --git a/yatube/core/tests.py b/yatube/core/tests.py index 64ea533..1a0ba38 100644 --- a/yatube/core/tests.py +++ b/yatube/core/tests.py @@ -1,6 +1,6 @@ from django.test import TestCase, Client - +# если это метод у тестированию, то он должен быть в ViewTestClass def setUp(self) -> None: self.guest_client = Client() diff --git a/yatube/core/views.py b/yatube/core/views.py index a9fefe6..a187b6b 100644 --- a/yatube/core/views.py +++ b/yatube/core/views.py @@ -6,6 +6,11 @@ def page_not_found(request, exception): # выводить её в шаблон пользовательской страницы 404 мы не станем return render(request, 'core/404.html', {'path': request.path}, status=404) - +# добавить сттус 403 def csrf_failure(request, reason=''): return render(request, 'core/403csrf.html') + +# должны быть еще вьюхи: +# 1. на обработку 500 ошибки, server_error +# 2. на обработку 403 ошибки, permission_denied +# соответственно, на обе вьюхи кастомные шаблоны \ No newline at end of file diff --git a/yatube/posts/admin.py b/yatube/posts/admin.py index 8ef6e01..bb7baac 100644 --- a/yatube/posts/admin.py +++ b/yatube/posts/admin.py @@ -11,7 +11,7 @@ class PostAdmin(admin.ModelAdmin): empty_value_display = '-пусто-' -admin.site.register(Post, PostAdmin) +admin.site.register(Post, PostAdmin) # Чтобы не выносить отдельно регистрацию модели, можно использовать декоратор- это предпочтительный способ. Особенно, когда моделей много. https://docs.djangoproject.com/en/4.0/ref/contrib/admin/#django.contrib.admin.register class GroupAdmin(admin.ModelAdmin): @@ -27,7 +27,7 @@ class CommentAdmin(admin.ModelAdmin): list_display = ('pk', 'post', 'author', 'text', 'created',) search_fields = ('text',) list_filter = ('author',) - search_fields = ('author', 'created') + search_fields = ('author', 'created') # дубль на 28 строке admin.site.register(Comment, CommentAdmin) diff --git a/yatube/posts/migrations/0002_auto_20230228_1135.py b/yatube/posts/migrations/0002_auto_20230228_1135.py index c1ba671..fe9c4dd 100644 --- a/yatube/posts/migrations/0002_auto_20230228_1135.py +++ b/yatube/posts/migrations/0002_auto_20230228_1135.py @@ -1,3 +1,5 @@ +# Миграции стоит называть осознанно(не xxxx_auto....), иначе в разросшихся миграциях будет непонятно, что и где было- будут сплошные авто-миграции. Если меняется нейминг миграций, уже примененных на БД, надо будет быть аккуратным- порядок миграций хранится в таблице (django_migrations) + в самих миграциях есть в зависимость на предыдущей миграции. Хорошая практика- 1 миграция на одну наработку. +# https://nextlinklabs.com/insights/naming-django-migrations-improving-projects  # Generated by Django 2.2.9 on 2023-02-28 11:35 from django.db import migrations, models diff --git a/yatube/posts/models.py b/yatube/posts/models.py index 14fef0c..5124c54 100644 --- a/yatube/posts/models.py +++ b/yatube/posts/models.py @@ -1,3 +1,4 @@ +# импорты не по pep8 from django.db import models from django.contrib.auth import get_user_model @@ -33,13 +34,13 @@ def __str__(self): return self.text[:15] class Meta: - ordering = ['-pub_date'] + ordering = ['-pub_date'] # Для неизменяемых последовательностей лучше использовать `tuple` https://www.programiz.com/python-programming/list-vs-tuples default_related_name = 'posts' class Group(models.Model): title = models.CharField(max_length=200) - slug = models.SlugField(max_length=50, null=False, unique=True) + slug = models.SlugField(max_length=50, null=False, unique=True) # не нужно указывать max_length для SlugField, по умолчанию он уже 50 символов. Указываем, только если нужно имзенить. `null=False,` уже идет по умолчанию. Лишний флаг description = models.TextField() def __str__(self): diff --git a/yatube/posts/urls.py b/yatube/posts/urls.py index e3d7349..f1d7f04 100644 --- a/yatube/posts/urls.py +++ b/yatube/posts/urls.py @@ -1,8 +1,11 @@ +# импорты не по pep8 from django.urls import path from . import views app_name = 'posts' + +# удали учебные комментарии urlpatterns = [ # Главная страница path('', views.index, name='index'), diff --git a/yatube/posts/views.py b/yatube/posts/views.py index 0beeb88..e530275 100644 --- a/yatube/posts/views.py +++ b/yatube/posts/views.py @@ -1,3 +1,4 @@ +# импорты не по зуз8 from django.contrib.auth.decorators import login_required from django.core.paginator import Paginator @@ -8,24 +9,24 @@ def index(request): - post_list = Post.objects.all().select_related('author') + post_list = Post.objects.all().select_related('author') # стоит также сделать связь с `group`, чтобы избежать лишних хинтов в БД page_obj = paginator(request, post_list) - title = 'Главная страница сайта Yatube' + title = 'Главная страница сайта Yatube' # Не стоит превращать view в передачу статичных аргументов в шаблон. Используй `block title` для переопределения нужного блока. https://docs.djangoproject.com/en/4.0/ref/templates/language/#templates context = { 'page_obj': page_obj, 'title': title } - template = 'posts/index.html' + template = 'posts/index.html' # Нет необходимости выносить в отдельную переменную template, можно сразу прокинуть шаблон в render() вторым аргументом- читабельность не потеряется. return render(request, template, context) def group_posts(request, slug): group = get_object_or_404(Group, slug=slug) - posts = group.posts.all()[:10] + posts = group.posts.all()[:10] # обрезка постов должна быть в отпагинированном объекте `page_obj`. Сейчас же в пагинацию не попадет никогда больше 10 постов группы page_obj = paginator(request, posts) context = { 'group': group, - 'posts': posts, + 'posts': posts, # вот это лишнее. Все объекты передаем в `page_obj`, отпагинированными. 'page_obj': page_obj } template = 'posts/group_list.html' @@ -33,14 +34,15 @@ def group_posts(request, slug): def profile(request, username): - author = User.objects.get(username=username) - post_list = author.posts.all() + author = User.objects.get(username=username) # Используй `get_object_or_404(...)`, чтобы в случае отсутствия модели была 404 страничка. https://docs.djangoproject.com/en/4.0/topics/http/shortcuts/#get-object-or-404 + post_list = author.posts.all() # выборку принято называть с префиксом `queryset` `posts_queryset`, например. Тут же не список постов. Было бы здорово использовать `select_related` page_obj = paginator(request, post_list) - title = f'Профайл пользователя {username}' - all_posts = post_list.count() + title = f'Профайл пользователя {username}' # убрать в block tittle + all_posts = post_list.count() # дай подходящий нейминг. тут хранится `количество` постов + following = ( request.user.is_authenticated - and Follow.objects.filter( + and Follow.objects.filter( # выборка `Follow` должна проверять: подписан ли `текущий пользователь` на `автора`. Должно быть комплексное условие по двум полям author__following__user=request.user ).exists() ) @@ -58,7 +60,7 @@ def post_detail(request, post_id): post = get_object_or_404(Post, pk=post_id) posts_count = post.author.posts.count() context = { - 'title': post.text, + 'title': post.text, # пост уже передается, не нужно отдельно вытягивать его атрибуты во вьюхе. Это можно сделать прямо в б=шаблоне 'post_count': posts_count, 'post': post, 'comments': post.comments.all(), @@ -78,7 +80,7 @@ def post_create(request): return redirect('posts:profile', username=post.author) context = { 'form': form, - 'is_edit': False + 'is_edit': False # не обязательно передавать. Ошибки в шаблоне не будет, если переменной нет. } return render(request, 'posts/create_post.html', context) @@ -86,6 +88,12 @@ def post_create(request): @login_required def post_edit(request, post_id): post = get_object_or_404(Post, pk=post_id) + # Тут происходит лишний хинт в БД. Есть несколько вариантов избежать этого: + # + # - Использовать `select_related`, подгружая нужные поля заранее; + # - Сравнивать не модели, а их `id` `request.user !=edit_post.author_id` + # + # https://docs.djangoproject.com/en/4.1/ref/models/querysets/#select-related if request.user != post.author: return redirect('posts:post_detail', post_id) form = PostForm(request.POST or None, @@ -101,9 +109,9 @@ def post_edit(request, post_id): } return render(request, 'posts/create_post.html', context) - +# утилитам самое место в отдельном файле, например, `utils.py`. Во `views.py` только вьюхи def paginator(request, posts): - paginator = Paginator(posts, 10) + paginator = Paginator(posts, 10) # 10 вынеси в константу в settings.py, например page_number = request.GET.get('page') page_obj = paginator.get_page(page_number) return page_obj @@ -124,7 +132,7 @@ def add_comment(request, post_id): @login_required def follow_index(request): post_list = Post.objects.filter( - author__following__user=request.user or None + author__following__user=request.user or None # or None в выборке никак не поможет. Тут формируется queryset, а не форма ).all() page_obj = paginator(request, post_list) return render( @@ -137,7 +145,7 @@ def follow_index(request): @login_required def profile_follow(request, username): - author = User.objects.get(username=username) + author = User.objects.get(username=username) # get_object_or_404 if request.user != author: Follow.objects.get_or_create(user=request.user, author=author) return redirect('posts:profile', username=author.username) @@ -145,8 +153,8 @@ def profile_follow(request, username): @login_required def profile_unfollow(request, username): - author = User.objects.get(username=username) + author = User.objects.get(username=username) # get_object_or_404 following = Follow.objects.filter(user=request.user, author=author) - if following: + if following: # exists(), чтобы просто проверить существование объектов, а не тянуть их из БД following.delete() return redirect('posts:profile', username=author.username) diff --git a/yatube/templates/about/author.html b/yatube/templates/about/author.html index 24c9646..318610e 100644 --- a/yatube/templates/about/author.html +++ b/yatube/templates/about/author.html @@ -3,6 +3,7 @@ {% block content %}

Привет, я автор этого проекта

+{#Поработай над порядком в верстке. Принцип такой: каждый вложенный тег имеет на 1 табуляцию больше, чем родительский. Закрывающий тег на уровне с открывающим(по вертикали).#} {% load static %} @@ -11,7 +12,7 @@

Привет, я автор этого проекта

Меня зовут Ляховицкая Наталья.

-

+

По образованию я магистр изящных искусств.
По жизни разработчик
По факту просто хороший человек.
diff --git a/yatube/templates/about/tech.html b/yatube/templates/about/tech.html index 38d1f79..fab5ea4 100644 --- a/yatube/templates/about/tech.html +++ b/yatube/templates/about/tech.html @@ -5,6 +5,7 @@ {% comment %} Колонки с отступом сверху и снизу {% endcomment %}

Многое из того, что сделано, делалось долго.

+{#Оставлять комментарии в коде- плохой тон. Код стоит держать в чистоте.#} {% comment %} Боковой блок со списком технологий Займет всю ширину блока на мобильном diff --git a/yatube/templates/base.html b/yatube/templates/base.html index b8da30d..4018934 100644 --- a/yatube/templates/base.html +++ b/yatube/templates/base.html @@ -8,17 +8,17 @@ - + {# ссылки луче формировать через {% static ... $} https://docs.djangoproject.com/en/4.1/howto/static-files/ #} - {{ title }} + {{ title }} {# вот тут самое место для нового блока, block title #} -
+
{#тег уже есть в includes/header.html#} {% include 'includes/header.html' %}
@@ -32,7 +32,7 @@ -