Merge pull request #137 from profcomff/iss14/query-mark-lecturer

Zimovchik · web-flow · commit abaa5b7c687b · 2025-08-23T17:43:58.000+03:00
diff --git a/Makefile b/Makefile
@@ -14,6 +14,9 @@ format:
 	autoflake -r --in-place --remove-all-unused-imports ./migrations
 	isort ./migrations
 	black ./migrations
+	autoflake -r --in-place --remove-all-unused-imports ./tests
+	isort ./tests
+	black ./tests
 	
 db:
 	docker run -d -p 5432:5432 -e POSTGRES_HOST_AUTH_METHOD=trust --name db-rating_api postgres:15
diff --git a/rating_api/routes/lecturer.py b/rating_api/routes/lecturer.py
@@ -2,13 +2,21 @@
 
 from auth_lib.fastapi import UnionAuth
 from fastapi import APIRouter, Depends, Query
+from fastapi_filter import FilterDepends
 from fastapi_sqlalchemy import db
 from sqlalchemy import and_
 
 from rating_api.exceptions import AlreadyExists, ObjectNotFound
 from rating_api.models import Comment, Lecturer, LecturerUserComment, ReviewStatus
 from rating_api.schemas.base import StatusResponseModel
-from rating_api.schemas.models import CommentGet, LecturerGet, LecturerGetAll, LecturerPatch, LecturerPost
+from rating_api.schemas.models import (
+    CommentGet,
+    LecturerGet,
+    LecturerGetAll,
+    LecturerPatch,
+    LecturerPost,
+    LecturersFilter,
+)
 from rating_api.utils.mark import calc_weighted_mark
 
 
@@ -76,16 +84,11 @@ async def get_lecturer(id: int, info: list[Literal["comments", "mark"]] = Query(
 
 @lecturer.get("", response_model=LecturerGetAll)
 async def get_lecturers(
+    lecturer_filter=FilterDepends(LecturersFilter),
     limit: int = 10,
     offset: int = 0,
     info: list[Literal["comments", "mark"]] = Query(default=[]),
-    order_by: str = Query(
-        enum=["mark_weighted", "mark_kindness", "mark_freebie", "mark_clarity", "mark_general", "last_name"],
-        default="mark_weighted",
-    ),
-    subject: str = Query(''),
-    name: str = Query(''),
-    asc_order: bool = False,
+    mark: float = Query(default=None, ge=-2, le=2),
 ) -> LecturerGetAll:
     """
     `limit` - максимальное количество возвращаемых преподавателей
@@ -95,6 +98,13 @@ async def get_lecturers(
     `order_by` - возможные значения `"mark_weighted", "mark_kindness", "mark_freebie", "mark_clarity", "mark_general", "last_name"`.
     Если передано `'last_name'` - возвращается список преподавателей отсортированных по алфавиту по фамилиям
     Если передано `'mark_...'` - возвращается список преподавателей отсортированных по конкретной оценке
+    Если передано просто так (или с '+' в начале параметра), то сортирует по возрастанию
+    С '-' в начале -- по убыванию.
+
+    *Пример запросов с этим параметром*:
+    - `...?order_by=-mark_kindness`
+    - `...?order_by=mark_freebie`
+    - `...?order_by=+mark_freebie` (эквивалентно 2ому пункту)
 
     `info` - возможные значения `'comments'`, `'mark'`.
     Если передано `'comments'`, то возвращаются одобренные комментарии к преподавателю.
@@ -107,30 +117,17 @@ async def get_lecturers(
     `name`
     Поле для ФИО. Если передано `name` - возвращает всех преподователей, для которых нашлись совпадения с переданной строкой
 
-    `asc_order`
-    Если передано true, сортировать в порядке возрастания
-    Иначе - в порядке убывания
+    `mark`
+    Поле для оценки. Если передано, то возвращает только тех преподавателей, для которых средняя общая оценка ('general_mark')
+    больше, чем переданный 'mark'.
     """
-    lecturers_query = (
-        Lecturer.query(session=db.session)
-        .outerjoin(Lecturer.comments)
-        .group_by(Lecturer.id)
-        .filter(Lecturer.search_by_subject(subject))
-        .filter(Lecturer.search_by_name(name))
-        .order_by(
-            *(
-                Lecturer.order_by_mark(order_by, asc_order)
-                if "mark" in order_by
-                else Lecturer.order_by_name(order_by, asc_order)
-            )
-        )
+    lecturers_query = lecturer_filter.filter(
+        Lecturer.query(session=db.session).outerjoin(Lecturer.comments).group_by(Lecturer.id)
     )
-
+    lecturers_query = lecturer_filter.sort(lecturers_query)
     lecturers = lecturers_query.offset(offset).limit(limit).all()
     lecturers_count = lecturers_query.group_by(Lecturer.id).count()
 
-    if not lecturers:
-        raise ObjectNotFound(Lecturer, 'all')
     result = LecturerGetAll(limit=limit, offset=offset, total=lecturers_count)
     if "mark" in info:
         mean_mark_general = Lecturer.mean_mark_general()
@@ -143,6 +140,12 @@ async def get_lecturers(
                 for comment in db_lecturer.comments
                 if comment.review_status is ReviewStatus.APPROVED
             ]
+            if (
+                mark is not None
+                and approved_comments
+                and sum(comment.mark_general for comment in approved_comments) / len(approved_comments) <= mark
+            ):
+                continue
             if "comments" in info and approved_comments:
                 lecturer_to_result.comments = sorted(
                     approved_comments, key=lambda comment: comment.create_ts, reverse=True
@@ -166,6 +169,8 @@ async def get_lecturers(
             if approved_comments:
                 lecturer_to_result.subjects = list({comment.subject for comment in approved_comments})
         result.lecturers.append(lecturer_to_result)
+    if len(result.lecturers) == 0:
+        raise ObjectNotFound(Lecturer, 'all')
     return result
 
 
diff --git a/rating_api/schemas/models.py b/rating_api/schemas/models.py
@@ -1,10 +1,13 @@
 import datetime
+from typing import List
 from uuid import UUID
 
-from pydantic import field_validator
+from fastapi import Query
+from fastapi_filter.contrib.sqlalchemy import Filter
+from pydantic import ValidationInfo, field_validator
 
 from rating_api.exceptions import WrongMark
-from rating_api.models import ReviewStatus
+from rating_api.models import Lecturer, ReviewStatus
 from rating_api.schemas.base import Base
 
 
@@ -24,57 +27,13 @@ class CommentGet(Base):
     dislike_count: int
 
 
-class CommentGetWithStatus(Base):
-    uuid: UUID
-    user_id: int | None = None
-    create_ts: datetime.datetime
-    update_ts: datetime.datetime
-    subject: str | None = None
-    text: str
-    mark_kindness: int
-    mark_freebie: int
-    mark_clarity: int
-    mark_general: float
-    lecturer_id: int
+class CommentGetWithStatus(CommentGet):
     review_status: ReviewStatus
-    like_count: int
-    dislike_count: int
 
 
-class CommentGetWithAllInfo(Base):
-    uuid: UUID
-    user_id: int | None = None
-    create_ts: datetime.datetime
-    update_ts: datetime.datetime
-    subject: str | None = None
-    text: str
-    mark_kindness: int
-    mark_freebie: int
-    mark_clarity: int
-    mark_general: float
-    lecturer_id: int
+class CommentGetWithAllInfo(CommentGet):
     review_status: ReviewStatus
     approved_by: int | None = None
-    like_count: int
-    dislike_count: int
-
-
-class CommentPost(Base):
-    subject: str
-    text: str
-    create_ts: datetime.datetime | None = None
-    update_ts: datetime.datetime | None = None
-    mark_kindness: int
-    mark_freebie: int
-    mark_clarity: int
-    is_anonymous: bool = True
-
-    @field_validator('mark_kindness', 'mark_freebie', 'mark_clarity')
-    @classmethod
-    def validate_mark(cls, value):
-        if value not in [-2, -1, 0, 1, 2]:
-            raise WrongMark()
-        return value
 
 
 class CommentUpdate(Base):
@@ -92,22 +51,16 @@ def validate_mark(cls, value):
         return value
 
 
-class CommentImport(Base):
-    lecturer_id: int
-    subject: str | None = None
-    text: str
+class CommentPost(CommentUpdate):
     create_ts: datetime.datetime | None = None
     update_ts: datetime.datetime | None = None
-    mark_kindness: int
-    mark_freebie: int
-    mark_clarity: int
+    is_anonymous: bool = True
 
-    @field_validator('mark_kindness', 'mark_freebie', 'mark_clarity')
-    @classmethod
-    def validate_mark(cls, value):
-        if value not in [-2, -1, 0, 1, 2]:
-            raise WrongMark()
-        return value
+
+class CommentImport(CommentUpdate):
+    lecturer_id: int
+    create_ts: datetime.datetime | None = None
+    update_ts: datetime.datetime | None = None
 
 
 class CommentImportAll(Base):
@@ -123,16 +76,10 @@ class CommentGetAll(Base):
 
 class CommentGetAllWithStatus(Base):
     comments: list[CommentGetWithStatus] = []
-    limit: int
-    offset: int
-    total: int
 
 
 class CommentGetAllWithAllInfo(Base):
     comments: list[CommentGetWithAllInfo] = []
-    limit: int
-    offset: int
-    total: int
 
 
 class LecturerUserCommentPost(Base):
@@ -171,9 +118,63 @@ class LecturerPost(Base):
     timetable_id: int | None = None
 
 
-class LecturerPatch(Base):
+class LecturerPatch(LecturerPost):
     first_name: str | None = None
     last_name: str | None = None
     middle_name: str | None = None
-    avatar_link: str | None = None
-    timetable_id: int | None = None
+
+
+class LecturersFilter(Filter):
+    subject: str = ''
+    name: str = ''
+    order_by: List[str] = [
+        'mark_weighted',
+    ]
+
+    @field_validator("*", mode="before", check_fields=False)
+    def validate_order_by(cls, value, field: ValidationInfo):
+        return value
+
+    @field_validator('order_by', mode='before')
+    def check_order_param(cls, value: str) -> str:
+        """Проверяет, что значение поля (без +/-) входит в список возможных."""
+        allowed_ordering = {
+            "mark_weighted",
+            "mark_kindness",
+            "mark_freebie",
+            "mark_clarity",
+            "mark_general",
+            "last_name",
+        }
+        cleaned_value = value.replace("+", "").replace("-", "")
+        if cleaned_value in allowed_ordering:
+            return value
+        else:
+            raise ValueError(f'"order_by"-field must contain value from {allowed_ordering}.')
+
+    def filter(self, query: Query) -> Query:
+        if self.subject:
+            query = query.filter(self.Constants.model.search_by_subject(self.subject))
+        if self.name:
+            query = query.filter(self.Constants.model.search_by_name(self.name))
+        return query
+
+    def sort(self, query: Query) -> Query:
+        if not self.ordering_values:
+            return query
+        elif len(self.ordering_values) > 1:
+            raise ValueError('order_by (хотя бы пока что) поддерживает лишь один параметр для сортировки!')
+
+        for field_name in self.ordering_values:
+            direction = True
+            if field_name.startswith("-"):
+                direction = False
+            field_name = field_name.replace("-", "").replace("+", "")
+            if field_name.startswith('mark_'):
+                query = query.order_by(*self.Constants.model.order_by_mark(field_name, direction))
+            else:
+                query = query.order_by(*self.Constants.model.order_by_name(field_name, direction))
+            return query
+
+    class Constants(Filter.Constants):
+        model = Lecturer
diff --git a/requirements.txt b/requirements.txt
@@ -3,6 +3,7 @@ auth-lib-profcomff[fastapi]
 aiohttp
 fastapi
 fastapi-sqlalchemy
+fastapi-filter[sqlalchemy]
 gunicorn
 logging-profcomff
 psycopg2-binary
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -221,6 +221,7 @@ def lecturers_with_comments(dbsession, lecturers):
       with 6 comments to non-deleted lecturers 4 approved and one dismissed and one pending.
         Two of them have alike names.
         Two of them have a different user_id.
+        One of them have a different subject.
     """
     comments_data = [
         (lecturers[0].id, 9990, 'test_subject', ReviewStatus.APPROVED, 1, 1, 1),
@@ -240,7 +241,7 @@ def lecturers_with_comments(dbsession, lecturers):
         (lecturers[2].id, 9990, 'test_subject2', ReviewStatus.DISMISSED, 2, 2, 2),
         (lecturers[2].id, 9990, 'test_subject2', ReviewStatus.PENDING, -2, -2, -2),
         (lecturers[2].id, 9991, 'test_subject11', ReviewStatus.APPROVED, 1, 1, 1),
-        (lecturers[2].id, 9992, 'test_subject12', ReviewStatus.APPROVED, 0, 0, 0),
+        (lecturers[2].id, 9992, 'test_subject13', ReviewStatus.APPROVED, 0, 0, 0),
     ]
 
     comments = []
diff --git a/tests/test_routes/test_lecturer.py b/tests/test_routes/test_lecturer.py
