Backend review by IAmLamarr · Pull Request #12 · saneczkab/task-tracker-service

IAmLamarr · 2025-11-19T13:14:39Z

No description provided.

IAmLamarr

Общие замечания

Нет линтеров / форматтеров => местами нарушается код-стайл
Рекомендую поставить и сконфигурировать ruff
Нет README.md, хотелось бы видеть базовое описание проекта, как его запускать
Нет .env-example, непонятно какие переменные среды нужно заполнить чтобы стартовать проект, нужно ознакамливаться со всей кодовой базой перед запуском
Не зафиксирована версия python, на которой работает проект
Можно зафиксировать в README.md или использовать poetry или uv для менеджмента зависимостей и версий python
Нет Dockerfile
"Слишком умные" роутеры, не реализовано разделение "сервис - репозиторий", из-за этого сильно дублируется код, связанный с получением тех или иных сущностей
Некорректная работа с роутерами
Нужно выделять общие роуты через префиксы, это уменьшит количество ошибок при создании URL + даст возможность реализовывать свои dependencies в роутах, уменьшать дублирование логики
Магические числа в части работы с ролями
Если хочется проверить наличие роли в рамках команды, нужно завязываться на name роли (а еще лучше сделать отдельный dependency для этой задачи)
Коды ответов HTTP проще указывать цифрой, а не импортировать из fastapi
Коды везде одинаковые, не вижу смысла в импорте; Это уменьшит количество лишнего кода
Сложно читать из-за импортов
Нужно стараться использовать конструкцию from ... import ...
Пример:

import typing
import project_schemas
import user
import fastapi
import orm
import team
import project

...

@router.get("/api/team/{team_id}/projects", response_model=typing.List[project_schemas.ProjectResponse],
            status_code=fastapi.status.HTTP_200_OK)
def get_team_projects(team_id: int, current_user: user.User = fastapi.Depends(auth.get_current_user),
                      data_base: orm.Session = fastapi.Depends(db.get_db)):
    """Получить все проекты в команде team_id"""
    team_obj = data_base.query(team.Team).filter(team.Team.id == team_id).first()

    if not team_obj:
        raise fastapi.HTTPException(status_code=fastapi.status.HTTP_404_NOT_FOUND, detail="Команда не найдена")

    user_team = data_base.query(team.UserTeam).filter(team.UserTeam.user_id == current_user.id,
                                                      team.UserTeam.team_id == team_id).first()

    if not user_team:
        raise fastapi.HTTPException(status_code=fastapi.status.HTTP_403_FORBIDDEN,
                                    detail="У вас нет доступа к этой команде")

    projects = data_base.query(project.Project).filter(project.Project.team_id == team_id).all()

    return projects

Используем from import:

from typing import List
from project_schemas import ProjectResponse
from user import User
from fastapi import Depends, HTTPException
from orm import Session
from team import Team, UserTeam
from project import Project

...

@router.get("/api/team/{team_id}/projects", response_model=List[ProjectResponse],
            status_code=200)
def get_team_projects(team_id: int, current_user: User = Depends(auth.get_current_user),
                      data_base: Session = Depends(db.get_db)):
    """Получить все проекты в команде team_id"""
    team_obj = data_base.query(Team).filter(Team.id == team_id).first()

    if not team_obj:
        raise HTTPException(status_code=404, detail="Команда не найдена")

    user_team = data_base.query(UserTeam).filter(UserTeam.user_id == current_user.id,
                                                      UserTeam.team_id == team_id).first()

    if not user_team:
        raise HTTPException(status_code=403,
                                    detail="У вас нет доступа к этой команде")

    projects = data_base.query(Project).filter(Project.team_id == team_id).all()

    return projects

Второй вариант читается гораздо легче, т.к модули не упоминаются множество раз в коде

Подумать, точно ли нужны точечные сообщения для каждой ошибки или будет достаточно общих
Можно ли отдавать в get_team_projects в случае 403 сообщение "Недостаточно прав" вместо "У вас нет доступа к этой команде"
Если да, то можно вынести сделать реестр общих ошибок и выдавать их оттуда, вместо того, чтобы каждый раз писать логику для каждого роута

IAmLamarr · 2025-11-19T13:16:29Z

gantt-backend/.gitignore

Неполный gitignore, может не подойти для другой IDE
Игноры можно брать отсюда, в данном случае нужен игнор для "Python"

IAmLamarr · 2025-11-19T13:18:23Z

gantt-backend/app/api/auth.py

+from app.core import db, security
+from app.models import user
+
+router = fastapi.APIRouter()


Указать тег auth?

IAmLamarr · 2025-11-19T13:26:08Z

gantt-backend/app/api/auth.py

+    try:
+        data_base.commit()
+        data_base.refresh(u)
+    except exc.IntegrityError:
+        data_base.rollback()
+        raise fastapi.HTTPException(status_code=fastapi.status.HTTP_409_CONFLICT,
+                                    detail="Пользователь с таким email или именем уже существует")


Не совсем корректно обрабатывать так проверку уникальности email и nickname
Если в будущем добавится новое поле с unique и нарушится его уникальность, вернется ошибка с тем что, что-то не то с email/nickname

Лучше запросить юзера из БД по такому email или такому nickname и если что-то нашлось - выдавать ошибку
Если не нашлось - добавлять нового юзера

IAmLamarr · 2025-11-19T13:39:35Z

gantt-backend/app/api/goal.py

+    if not stream_obj:
+        raise fastapi.HTTPException(status_code=fastapi.status.HTTP_404_NOT_FOUND, detail="Стрим не найден")
+
+    project_obj = data_base.query(project.Project).filter(project.Project.id == stream_obj.project_id).first()


Нет проверки project_obj по аналогии с stream_obj и user_team

IAmLamarr · 2025-11-19T13:40:09Z

gantt-backend/app/api/goal.py

+    if not stream_obj:
+        raise fastapi.HTTPException(status_code=fastapi.status.HTTP_404_NOT_FOUND, detail="Стрим не найден")
+
+    project_obj = data_base.query(project.Project).filter(project.Project.id == stream_obj.project_id).first()


Нет проверки project_obj

IAmLamarr · 2025-11-19T15:27:46Z

gantt-backend/app/models/base.py

+
+Base = declarative_base()


Можно вынести id в базовую модель

IAmLamarr · 2025-11-19T15:29:17Z

gantt-backend/app/models/meta.py

+class UserTask(base.Base):
+    __tablename__ = "UserTask"
+    id = Column(Integer, primary_key=True)
+    user_id = Column(Integer, ForeignKey("Users.id"), nullable=False)
+    task_id = Column(Integer, ForeignKey("Tasks.id"), nullable=False)
+
+    user = relationship("User", backref="user_tasks")
+    task = relationship("Task", backref="user_tasks")


Кажется, это должно быть в models/task.py

IAmLamarr · 2025-11-19T15:36:20Z

gantt-backend/app/schemas/team.py

+class UserRole(str, Enum):
+    READER = "Reader"
+    EDITOR = "Editor"


Где-то выше уже есть такой enum

IAmLamarr · 2025-11-19T15:37:45Z

gantt-backend/app/schemas/user.py

+    @field_validator("email")
+    def validate_email(cls, value):
+        if not value:
+            raise fastapi.HTTPException(status_code=400, detail="Некорректный формат email")
+        return value


А что не так со стандартной валидацией?

IAmLamarr · 2025-11-19T15:41:45Z

gantt-backend/app/tests/conftest.py

+@pytest.fixture(scope="session", autouse=True)
+def load_env_and_models():
+    env_path = Path(__file__).parent.parent.parent / '.env'
+    load_dotenv(env_path)
+
+    import app.models.goal
+    import app.models.task
+    import app.models.meta


Почему происходит импорт внутри функции? Это плохая практика

В load_dotenv кажется можно не подавать такой путь, он по умолчанию будет искать .env в той директории, откуда запускается проект

maksimzaitsevV and others added 30 commits October 27, 2025 15:47

Implement FastAPI backend with user auth and models

3eb6a5e

Fix bugs

da88713

Fix bugs

c0cc424

Add auth processing to frontend

a8bb305

Implement backend for teams, streams, goals and projects

eb13de2

fix bugs

3a87613

Fix token bug

bf845a6

Fix database bug

a1af5da

Create sidebar

d1885b7

Create Topbar

1010ba5

Create TeamPage

c57442c

Remove unused imports

b81bfae

Implement backend for task and meta

38a65ca

Fix multiple Base bug

ffad3cb

Add get stream by id func

7adeef5

Fix meta.py table names

443c9c0

Fix task bugs

1272536

Add timestamp and assignees fields to Task

c25f987

Fix assignee and datetime bugs

d32e365

Implement forms

cca7fe3

Implement goal and task lists

d10beac

Create stream page

313e4e1

Status and priority id fix

0678ec8

Create FormRow

322d30e

Remove unused Row from TaskForm

2ac29c7

Refactor

ff39053

Update return in goal and task lists

01771c7

Create datetime utils module

12c6755

Remove unused files

dc1acf7

Add font link

b64f76f

saneczkab and others added 9 commits November 11, 2025 23:19

Table style update

c2ea45b

Implement tests and refactoring

23da4b4

fix bugs

885dd29

fix bugs

5a19863

Fix create_task id and update_task assignee bugs

817ce4a

Update font

5e10a25

Create kanban board

3c50bd0

Separate api fetch

d10da55

Fix formatting

27ec403

IAmLamarr commented Nov 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Backend review#12

Backend review#12
IAmLamarr wants to merge 39 commits intomainfrom
frontend_prettier

IAmLamarr commented Nov 19, 2025

Uh oh!

IAmLamarr left a comment

Uh oh!

IAmLamarr Nov 19, 2025

Uh oh!

IAmLamarr Nov 19, 2025

Uh oh!

IAmLamarr Nov 19, 2025

Uh oh!

IAmLamarr Nov 19, 2025

Uh oh!

IAmLamarr Nov 19, 2025

Uh oh!

IAmLamarr Nov 19, 2025

Uh oh!

IAmLamarr Nov 19, 2025

Uh oh!

IAmLamarr Nov 19, 2025

Uh oh!

IAmLamarr Nov 19, 2025

Uh oh!

IAmLamarr Nov 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		Base = declarative_base() No newline at end of file

Comments

Conversation

IAmLamarr commented Nov 19, 2025

Uh oh!

IAmLamarr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants