-
Notifications
You must be signed in to change notification settings - Fork 2
Temmmmmo patch 2 #110
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
base: main
Are you sure you want to change the base?
Temmmmmo patch 2 #110
Conversation
💩 Code linting failed, use |
💩 Code linting failed, use |
💩 Code linting failed, use |
💩 Code linting failed, use |
super().__init__(eng) | ||
|
||
|
||
class ObjectNotFound(RatingAPIError): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[КРИТИЧНО] Конструктор класса не инициализирует базовый класс
@classmethod | ||
def delete(cls, id: int | str, *, session: Session) -> None: | ||
"""Soft delete object if possible, else hard delete""" | ||
obj = cls.get(id, session=session) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[КРИТИЧНО] Логическая ошибка в обновлении атрибутов объекта**
if not with_deleted and hasattr(cls, "is_deleted"): | ||
objs = objs.filter(not_(cls.is_deleted)) | ||
try: | ||
if hasattr(cls, "uuid"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[КРИТИЧНО] Логическая ошибка в методе удаления объекта**
if not with_deleted and hasattr(cls, "is_deleted"): | ||
objs = objs.filter(not_(cls.is_deleted)) | ||
try: | ||
if hasattr(cls, "uuid"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[СТИЛЬ] Избыточный вывод информации**
# raise ForbiddenSymbol() | ||
|
||
# Сначала добавляем с user_id, который мы получили при авторизации, | ||
# в LecturerUserComment, чтобы нельзя было слишком быстро добавлять комментарии |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[КРИТИЧНО] Закомментированный код проверки на запрещенные символы
# Обрабатываем анонимность комментария, и удаляем этот флаг чтобы добавить запись в БД | ||
user_id = None if comment_info.is_anonymous else user.get('id') | ||
|
||
new_comment = Comment.create( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[УЛУЧШЕНИЕ] Удаление параметра session из вызова метода create
if response.status == 300: | ||
user_achievements = await response.json() | ||
for achievement in user_achievements.get("achievement", []): | ||
if achievement.get("id") == settings.FIRST_COMMENT_ACHIEVEMENT_ID: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[КРИТИЧНО] Изменение статуса ответа с 200 на 300
|
||
@app.exception_handler(ObjectNotFound) | ||
@app.exception_handler(AlreadyExists) | ||
async def not_found_handler(req: starlette.requests.Request, exc: ObjectNotFound): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[КРИТИЧНО] Несоответствие типа исключения и обработчика
|
||
@app.exception_handler(ObjectNotFound) | ||
@app.exception_handler(AlreadyExists) | ||
async def not_found_handler(req: starlette.requests.Request, exc: ObjectNotFound): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[УЛУЧШЕНИЕ] Использование неправильного статуса ответа
text: str | ||
mark_kindness: int | ||
mark_freebie: int | ||
mark_clarity: int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[СТИЛЬ] Отсутствует документация класса
super().__init__(eng) | ||
|
||
|
||
class ObjectNotFound(RatingAPIError): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[КРИТИЧНО] Удалено сообщение об ошибке
else: | ||
session.delete(obj) | ||
print(cls.id, cls.session, session) | ||
session.flush() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[КРИТИЧНО] Неправильное использование метода setattr
if not with_deleted and hasattr(cls, "is_deleted"): | ||
objs = objs.filter(not_(cls.is_deleted)) | ||
try: | ||
if hasattr(cls, "uuid"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[КРИТИЧНО] Удаление объекта из сессии не выполняется
if not with_deleted and hasattr(cls, "is_deleted"): | ||
objs = objs.filter(not_(cls.is_deleted)) | ||
try: | ||
if hasattr(cls, "uuid"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[СТИЛЬ] Лишний вывод в консоль
settings.COMMENT_LECTURER_FREQUENCE_IN_MONTH, settings.COMMENT_TO_LECTURER_LIMIT | ||
) | ||
|
||
if len(comment_info.text) > settings.MAX_COMMENT_LENGTH: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[КРИТИЧНО] Проверка на запрещенные символы закомментирована
settings.COMMENT_LECTURER_FREQUENCE_IN_MONTH, settings.COMMENT_TO_LECTURER_LIMIT | ||
) | ||
|
||
if len(comment_info.text) > settings.MAX_COMMENT_LENGTH: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[ВАЖНО] Удаление параметра session при создании комментария
async with session.get( | ||
settings.API_URL + f"achievement/user/{user.get('id'):}", | ||
headers={"Accept": "application/json"}, | ||
) as response: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[КРИТИЧНО] Неправильная проверка статуса ответа
|
||
|
||
@app.exception_handler(ObjectNotFound) | ||
@app.exception_handler(AlreadyExists) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[КРИТИЧНО] Несоответствие типа исключения
|
||
@app.exception_handler(ObjectNotFound) | ||
@app.exception_handler(AlreadyExists) | ||
async def not_found_handler(req: starlette.requests.Request, exc: ObjectNotFound): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[СТИЛЬ] Неправильный статус кода
update_ts: datetime.datetime | ||
subject: str | None = None | ||
text: str | ||
mark_kindness: int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[УЛУЧШЕНИЕ] Удаление полей без комментария
Изменения
Детали реализации
Check-List
black
иisort
для Back-End илиPrettier
для Front-End?