-
Notifications
You must be signed in to change notification settings - Fork 3
Iss14/query mark lecturer #137
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
Conversation
|
💩 Code linting failed, use |
Coverage Report
Summary
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Zimovchik
left a comment
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.
Со схемами респорсов, я не против того, чтобы использовать наследование там, но давай только одноуровневое. Тоесть если у тебя есть базовы CommentGet, то остальные схожие пусть наследуются от него только, а не друг от друга и тд. Наследование правда упростит чуть чуть код, но если переборщим, мне кажется уже будет не так читаемо. А еще надо проверить, чтобы в сваггере все норм отображалось
rating_api/routes/comment.py
Outdated
| give_achievement = False | ||
| if give_achievement: | ||
| session.post( | ||
| await session.post( |
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.
зачем
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.
Наверное, стоило в беседе сначала спросить... Просто при запуске тестов я увидел следующее сообщение на эту строку:
RuntimeWarning: coroutine 'ClientSession._request' was never awaited
session.post(
Честно говоря, метод-то объявлен как синхронный, но возвращает асинхронный объект. В целом, тесты в любом случае отрабатывают, а в свагере я не тестил эту ручку. Тч я пока что править не буду, но если что расклад такой
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.
просто это выдача ачивки и нам для завершения выполнения ручки явно не нужно ждать, что этот процесс завершится, то зафейлится оно или выполнится успешно не должно влиять на основную работу ручки. В тестах оно выдает ошибку, потому что ты не на тестовом или продовом стенде их запускаешь, там все норм. В идеале бы конечно просто локально не выдавать эту ачивку, но пока это не сделали
rating_api/routes/lecturer.py
Outdated
| lecturers_query = lecturer_filter.sort(lecturers_query) | ||
| lecturers_query = lecturer_filter.sort(lecturers_query) |
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.
почему 2 раза?
| if ( | ||
| mark is not None | ||
| and approved_comments | ||
| and sum(comment.mark_general for comment in approved_comments) / len(approved_comments) < mark | ||
| ): | ||
| continue |
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.
поясни зачем это
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.
Это логика для параметра mark из issue:
Если он передан (1ая строка) и есть комментарии, по которым можно считать оценку (2ая строка), то проверяем, что средняя (по approved_comments) mark_general лектора не меньше переданного параметра. Если меньше, то не добавляем этого лектора в список result (который и возвращается пользователю) и просто переходим к следующему (continue в цикле по лекторам)
|
а еще когда марк указывается, то давай нестрогое сравнение |
Зачем? Просто по заданию надо возвращать лекторов "для которых оценка не ниже mark", что я и делаю (в условной конструкции сравнение строго, так как оно работает на отсеивание лекторов). |
с точки зрения юзера намного логичнее делать нестрогое сравнение, например если я хотел бы получить всех лекторов с оценкой 2.0. + Фронты сказали, им так будет понятнее и удобнее |
Изменения
Детали реализации
Check-List
blackиisortдля Back-End илиPrettierдля Front-End?