diff --git a/Makefile b/Makefile index 8a826f9..fe8b677 100644 --- 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 index 64632fa..7ec9f70 100644 --- 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 index da53351..d74e938 100644 --- 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 index 920ab75..17be111 100644 --- 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 index b887bbc..32bc43d 100644 --- 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 index 95d5aca..8c42600 100644 --- a/tests/test_routes/test_lecturer.py +++ b/tests/test_routes/test_lecturer.py @@ -1,10 +1,13 @@ import logging import pytest +from fastapi_sqlalchemy import db +from sqlalchemy import and_, func, select from starlette import status -from rating_api.models import Lecturer +from rating_api.models import Comment, Lecturer, ReviewStatus from rating_api.settings import get_settings +from rating_api.utils.mark import calc_weighted_mark logger = logging.getLogger(__name__) @@ -96,6 +99,201 @@ def test_get_lecturers_by_name(client, lecturers, query, total, response_status) assert json_response["lecturers"][0]["first_name"] == lecturers[0].first_name +@pytest.mark.usefixtures('lecturers_with_comments') +@pytest.mark.parametrize( + 'query, response_status', + [ + ({'subject': 'test_subject'}, status.HTTP_200_OK), + ({'subject': 'test_subject13'}, status.HTTP_200_OK), + ({'subject': 'test_subject666'}, status.HTTP_404_NOT_FOUND), + ], + ids=[ + 'get_all', + 'get_some', + 'wrong_subject', + ], +) +def test_get_lecturers_by_subject(client, dbsession, query, response_status): + """ + Проверка, что при передаче subject возвращаются только лекторы, + в одобренных комментариях к которым есть поле subject, совпадающее с переданным. + """ + resp = client.get(f'{url}', params=query) + assert resp.status_code == response_status + if response_status == status.HTTP_200_OK: + db_lecturers = { + comment.lecturer_id + for comment in Comment.query(session=dbsession).filter( + and_(Comment.review_status == ReviewStatus.APPROVED, Comment.subject == query['subject']) + ) + } + resp_lecturers = {lecturer['id'] for lecturer in resp.json()['lecturers']} + assert resp_lecturers == db_lecturers + + +@pytest.mark.usefixtures('lecturers_with_comments') +@pytest.mark.parametrize( + 'query, response_status', + [ + ({'mark': -2}, status.HTTP_200_OK), + ({'mark': 0}, status.HTTP_200_OK), + ({'mark': 2}, status.HTTP_404_NOT_FOUND), + ({'mark': -3}, status.HTTP_422_UNPROCESSABLE_ENTITY), + ({'mark': 3}, status.HTTP_422_UNPROCESSABLE_ENTITY), + ], + ids=['get_all', 'get_some', 'get_nothing', 'under_min', 'above_max'], +) +def test_get_lecturers_by_mark(client, dbsession, query, response_status): + """ + Проверка, что при передаче mark возвращаются только лекторы + со средним mark_general (по комментариям) не меньше, чем переданный mark. + """ + resp = client.get(f'{url}', params=query) + assert resp.status_code == response_status + if response_status == status.HTTP_200_OK: + res = dbsession.execute( + ( + select(Lecturer.id.label('lecturer'), func.avg(Comment.mark_general).label('avg')) + .join(Comment, and_(Comment.review_status == ReviewStatus.APPROVED, Lecturer.id == Comment.lecturer_id)) + .group_by(Lecturer.id) + .having(func.avg(Comment.mark_general) > query['mark']) + ) + ).all() + resp_lecturers = {lecturer['id'] for lecturer in resp.json()['lecturers']} + db_lecturers = {req[0] for req in res} + assert resp_lecturers == db_lecturers, 'Убедитесь, что все подходящие лекторы отправляются пользователю!' + + +@pytest.mark.usefixtures('lecturers_with_comments') +@pytest.mark.parametrize( + 'query, response_status', + [ + ({'info': ['comments', 'mark']}, status.HTTP_200_OK), + ({'info': ['comments']}, status.HTTP_200_OK), + ({'info': ['mark']}, status.HTTP_200_OK), + ({'info': []}, status.HTTP_200_OK), + ({'info': {}}, status.HTTP_422_UNPROCESSABLE_ENTITY), + ({'info': ['pupupu']}, status.HTTP_422_UNPROCESSABLE_ENTITY), + ], + ids=["comments_and_marks", "only_comments", "only_marks", "no_info", "invalid_iterator", "invalid_param"], +) +def test_get_lecturers_by_info(client, dbsession, query, response_status): + """ + Проверка, что при передаче info разного состава возвращаются лекторы + с полями комментариев и/или оценок на основе них для каждого лектора. + """ + resp = client.get(f'{url}', params=query) + assert resp.status_code == response_status + if response_status == status.HTTP_200_OK: + if 'mark' in query['info']: + db_res = dbsession.execute( + ( + select( + Lecturer.id.label('lecturer'), + func.avg(Comment.mark_freebie).label('avg_freebie'), + func.avg(Comment.mark_kindness).label('avg_kindness'), + func.avg(Comment.mark_clarity).label('avg_clarity'), + func.avg(Comment.mark_general).label('avg_general'), + ) + .join( + Comment, + and_(Comment.review_status == ReviewStatus.APPROVED, Lecturer.id == Comment.lecturer_id), + ) + .group_by(Lecturer.id) + ) + ).all() + with db(): + mean_mark_general = Lecturer.mean_mark_general() + db_lecturers = { + ( + *lecturer, + calc_weighted_mark( + float(lecturer[-1]), + Comment.query(session=dbsession) + .filter( + and_(Comment.review_status == ReviewStatus.APPROVED, Comment.lecturer_id == lecturer[0]) + ) + .count(), + mean_mark_general, + ), + ) + for lecturer in db_res + } + resp_lecturers = { + ( + lecturer['id'], + lecturer['mark_freebie'], + lecturer['mark_kindness'], + lecturer['mark_clarity'], + lecturer['mark_general'], + lecturer['mark_weighted'], + ) + for lecturer in resp.json()['lecturers'] + } + assert resp_lecturers == db_lecturers + if 'comments' in query['info']: + db_res = dbsession.execute( + ( + select(Lecturer.id.label('lecturer'), func.count(Comment.uuid)) + .join( + Comment, + and_(Comment.review_status == ReviewStatus.APPROVED, Lecturer.id == Comment.lecturer_id), + ) + .group_by(Lecturer.id) + ) + ).all() + db_lecturers = {*db_res} + assert len(resp.json()['lecturers']) == len(db_lecturers) + for lecturer in resp.json()['lecturers']: + assert (lecturer['id'], len(lecturer['comments'])) in db_lecturers + + +@pytest.mark.usefixtures('lecturers_with_comments') +@pytest.mark.parametrize( + 'query, response_status', + [ + ({'order_by': 'mark_kindness'}, status.HTTP_200_OK), + ({}, status.HTTP_200_OK), + ({'order_by': '+mark_freebie'}, status.HTTP_200_OK), + ({'order_by': '-mark_clarity'}, status.HTTP_200_OK), + ({'order_by': 'pupupu'}, status.HTTP_422_UNPROCESSABLE_ENTITY), + ({'order_by': 'mark_kindness,mark_freebie'}, status.HTTP_422_UNPROCESSABLE_ENTITY), + ], + ids=["valid", "valid_default", "valid_plus", "valid_minus", "invalid_param", "invalid_many_params"], +) +def test_get_lecturers_order_by(client, dbsession, query, response_status): + """ + Проверка, что при передаче (или нет) параметра order_by возвращаемый + список лекторов верно сортируется. + """ + resp = client.get(f'{url}', params=query) + assert resp.status_code == response_status + if response_status == status.HTTP_200_OK: + if 'order_by' not in query: + field_name = 'mark_weighted' + asc_order = True + elif query['order_by'].startswith('+'): + field_name = query['order_by'][1:] + asc_order = True + elif query['order_by'].startswith('-'): + field_name = query['order_by'][1:] + asc_order = False + else: + field_name = query['order_by'] + asc_order = True + with db(): + db_res = ( + Lecturer.query(session=dbsession) + .join(Comment, and_(Comment.review_status == ReviewStatus.APPROVED, Lecturer.id == Comment.lecturer_id)) + .group_by(Lecturer.id) + .order_by(*Lecturer.order_by_mark(field_name, asc_order)) + .all() + ) + db_lecturers = [lecturer.id for lecturer in db_res] + resp_lecturers = [lecturer['id'] for lecturer in resp.json()['lecturers']] + assert resp_lecturers == db_lecturers + + @pytest.mark.parametrize( 'body,response_status', [