From b6252b2a568b94cc71f6505c96a08160c12d7cb2 Mon Sep 17 00:00:00 2001 From: Timur Enikeev Date: Sat, 9 Nov 2024 19:56:41 -0500 Subject: [PATCH 1/7] Fix --- rating_api/routes/lecturer.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/rating_api/routes/lecturer.py b/rating_api/routes/lecturer.py index 3be74fb..99c0cab 100644 --- a/rating_api/routes/lecturer.py +++ b/rating_api/routes/lecturer.py @@ -101,13 +101,11 @@ async def get_lecturers( `name` Поле для ФИО. Если передано `name` - возвращает всех преподователей, для которых нашлись совпадения с переданной строкой """ - lecturers = ( - Lecturer.query(session=db.session) - .join(Lecturer.comments) - .filter(Lecturer.search_by_name(name)) - .filter(Lecturer.search_by_subject(subject)) - .all() - ) + lecturers_query = Lecturer.query(session=db.session) + if subject: + lecturers_query = lecturers_query.join(Lecturer.comments).filter(Lecturer.search_by_subject(subject)) + lecturers_query = lecturers_query.filter(Lecturer.search_by_name(name)) + lecturers = lecturers_query.all() if not lecturers: raise ObjectNotFound(Lecturer, 'all') result = LecturerGetAll(limit=limit, offset=offset, total=len(lecturers)) From 279e6e549ab08d3b8d7d49f915092715c4b06458 Mon Sep 17 00:00:00 2001 From: Timur Enikeev Date: Sun, 10 Nov 2024 18:18:33 -0500 Subject: [PATCH 2/7] Fix lecturer sort --- rating_api/routes/lecturer.py | 36 +++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/rating_api/routes/lecturer.py b/rating_api/routes/lecturer.py index 99c0cab..ea786c4 100644 --- a/rating_api/routes/lecturer.py +++ b/rating_api/routes/lecturer.py @@ -3,7 +3,7 @@ from auth_lib.fastapi import UnionAuth from fastapi import APIRouter, Depends, Query from fastapi_sqlalchemy import db -from sqlalchemy import and_ +from sqlalchemy import and_, func, nullslast from rating_api.exceptions import AlreadyExists, ObjectNotFound from rating_api.models import Comment, Lecturer, LecturerUserComment, ReviewStatus @@ -76,19 +76,17 @@ async def get_lecturers( limit: int = 10, offset: int = 0, info: list[Literal["comments", "mark"]] = Query(default=[]), - order_by: list[Literal["general", '']] = Query(default=[]), + order_by: list[Literal["alphabet", '']] = Query(default=[]), subject: str = Query(''), name: str = Query(''), ) -> LecturerGetAll: """ - Scopes: `["rating.lecturer.read"]` - `limit` - максимальное количество возвращаемых преподавателей `offset` - нижняя граница получения преподавателей, т.е. если по дефолту первым возвращается преподаватель с условным номером N, то при наличии ненулевого offset будет возвращаться преподаватель с номером N + offset - `order_by` - возможные значения `'general'`. - Если передано `'general'` - возвращается список преподавателей отсортированных по общей оценке + `order_by` - возможные значения `'alphabet'`. + Если передано `'alphabet'` - возвращается список преподавателей отсортированных по алфавиту `info` - возможные значения `'comments'`, `'mark'`. Если передано `'comments'`, то возвращаются одобренные комментарии к преподавателю. @@ -102,14 +100,26 @@ async def get_lecturers( Поле для ФИО. Если передано `name` - возвращает всех преподователей, для которых нашлись совпадения с переданной строкой """ lecturers_query = Lecturer.query(session=db.session) - if subject: - lecturers_query = lecturers_query.join(Lecturer.comments).filter(Lecturer.search_by_subject(subject)) - lecturers_query = lecturers_query.filter(Lecturer.search_by_name(name)) - lecturers = lecturers_query.all() + lecturers_query = ( + lecturers_query.outerjoin(Lecturer.comments) + .group_by(Lecturer.id) + .filter(Lecturer.search_by_subject(subject)) + .filter(Lecturer.search_by_name(name)) + ) + if "alphabet" in order_by: + lecturers_query = lecturers_query.order_by(Lecturer.last_name) + else: + lecturers_query = lecturers_query.order_by( + nullslast(func.avg((Comment.mark_kindness + Comment.mark_freebie + Comment.mark_clarity) / 3).desc()) + ) + + 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=len(lecturers)) - for db_lecturer in lecturers[offset : limit + offset]: + result = LecturerGetAll(limit=limit, offset=offset, total=lecturers_count) + for db_lecturer in lecturers: lecturer_to_result: LecturerGet = LecturerGet.model_validate(db_lecturer) lecturer_to_result.comments = None if db_lecturer.comments: @@ -139,8 +149,6 @@ 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 "general" in order_by: - result.lecturers.sort(key=lambda item: (item.mark_general is None, item.mark_general)) return result From ca1b732ae2b2aeea3c733dee28518fa6afa1b812 Mon Sep 17 00:00:00 2001 From: Timur Enikeev Date: Mon, 11 Nov 2024 02:22:10 -0500 Subject: [PATCH 3/7] Add sort options --- rating_api/models/db.py | 6 +++++- rating_api/routes/lecturer.py | 15 ++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/rating_api/models/db.py b/rating_api/models/db.py index a25a820..12b651c 100644 --- a/rating_api/models/db.py +++ b/rating_api/models/db.py @@ -8,7 +8,7 @@ from sqlalchemy import UUID, Boolean, DateTime from sqlalchemy import Enum as DbEnum from sqlalchemy import ForeignKey, Integer, String, and_, func, or_, true -from sqlalchemy.ext.hybrid import hybrid_method +from sqlalchemy.ext.hybrid import hybrid_method, hybrid_property from sqlalchemy.orm import Mapped, mapped_column, relationship from rating_api.settings import get_settings @@ -74,6 +74,10 @@ class Comment(BaseDbModel): review_status: Mapped[ReviewStatus] = mapped_column(DbEnum(ReviewStatus, native_enum=False), nullable=False) is_deleted: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) + @hybrid_property + def mark_general(self): + return (self.mark_kindness + self.mark_freebie + self.mark_clarity) / 3 + class LecturerUserComment(BaseDbModel): id: Mapped[int] = mapped_column(Integer, primary_key=True) diff --git a/rating_api/routes/lecturer.py b/rating_api/routes/lecturer.py index 14fec3c..86dcff4 100644 --- a/rating_api/routes/lecturer.py +++ b/rating_api/routes/lecturer.py @@ -76,7 +76,9 @@ async def get_lecturers( limit: int = 10, offset: int = 0, info: list[Literal["comments", "mark"]] = Query(default=[]), - order_by: list[Literal["alphabet", '']] = Query(default=[]), + order_by: str = Query( + enum=["mark_kindness", "mark_freebie", "mark_clarity", "mark_general", "last_name"], default="mark_general" + ), subject: str = Query(''), name: str = Query(''), ) -> LecturerGetAll: @@ -105,13 +107,12 @@ async def get_lecturers( .group_by(Lecturer.id) .filter(Lecturer.search_by_subject(subject)) .filter(Lecturer.search_by_name(name)) - ) - if "alphabet" in order_by: - lecturers_query = lecturers_query.order_by(Lecturer.last_name) - else: - lecturers_query = lecturers_query.order_by( - nullslast(func.avg((Comment.mark_kindness + Comment.mark_freebie + Comment.mark_clarity) / 3).desc()) + .order_by( + nullslast(func.avg(getattr(Comment, order_by)).desc()) + if "mark" in order_by + else getattr(Lecturer, order_by) ) + ) lecturers = lecturers_query.offset(offset).limit(limit).all() lecturers_count = lecturers_query.group_by(Lecturer.id).count() From 433de63bc0b07aa86cd019f2130e045e086dec25 Mon Sep 17 00:00:00 2001 From: Timur Enikeev Date: Mon, 11 Nov 2024 02:25:39 -0500 Subject: [PATCH 4/7] Doc fix --- rating_api/routes/lecturer.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rating_api/routes/lecturer.py b/rating_api/routes/lecturer.py index 86dcff4..35ae49c 100644 --- a/rating_api/routes/lecturer.py +++ b/rating_api/routes/lecturer.py @@ -87,8 +87,9 @@ async def get_lecturers( `offset` - нижняя граница получения преподавателей, т.е. если по дефолту первым возвращается преподаватель с условным номером N, то при наличии ненулевого offset будет возвращаться преподаватель с номером N + offset - `order_by` - возможные значения `'alphabet'`. - Если передано `'alphabet'` - возвращается список преподавателей отсортированных по алфавиту + `order_by` - возможные значения `"mark_kindness", "mark_freebie", "mark_clarity", "mark_general", "last_name"`. + Если передано `'last_name'` - возвращается список преподавателей отсортированных по алфавиту по фамилиям + Если передано `'mark_...'` - возвращается список преподавателей отсортированных по конкретной оценке `info` - возможные значения `'comments'`, `'mark'`. Если передано `'comments'`, то возвращаются одобренные комментарии к преподавателю. From b80ca211c39dd4f6d8c4b68fcf018fd7ea873b00 Mon Sep 17 00:00:00 2001 From: Timur Enikeev Date: Mon, 11 Nov 2024 02:37:03 -0500 Subject: [PATCH 5/7] Minor fix for new hybrid.property --- rating_api/routes/lecturer.py | 14 +++++--------- rating_api/schemas/models.py | 1 + 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/rating_api/routes/lecturer.py b/rating_api/routes/lecturer.py index 35ae49c..58d627d 100644 --- a/rating_api/routes/lecturer.py +++ b/rating_api/routes/lecturer.py @@ -61,11 +61,10 @@ async def get_lecturer(id: int, info: list[Literal["comments", "mark"]] = Query( if "comments" in info and approved_comments: result.comments = approved_comments if "mark" in info and approved_comments: - result.mark_freebie = sum([comment.mark_freebie for comment in approved_comments]) / len(approved_comments) + result.mark_freebie = sum(comment.mark_freebie for comment in approved_comments) / len(approved_comments) result.mark_kindness = sum(comment.mark_kindness for comment in approved_comments) / len(approved_comments) result.mark_clarity = sum(comment.mark_clarity for comment in approved_comments) / len(approved_comments) - general_marks = [result.mark_freebie, result.mark_kindness, result.mark_clarity] - result.mark_general = sum(general_marks) / len(general_marks) + result.mark_general = sum(comment.mark_general for comment in approved_comments) / len(approved_comments) if approved_comments: result.subjects = list({comment.subject for comment in approved_comments}) return result @@ -142,12 +141,9 @@ async def get_lecturers( lecturer_to_result.mark_clarity = sum(comment.mark_clarity for comment in approved_comments) / len( approved_comments ) - general_marks = [ - lecturer_to_result.mark_freebie, - lecturer_to_result.mark_kindness, - lecturer_to_result.mark_clarity, - ] - lecturer_to_result.mark_general = sum(general_marks) / len(general_marks) + lecturer_to_result.mark_general = sum(comment.mark_general for comment in approved_comments) / len( + approved_comments + ) if approved_comments: lecturer_to_result.subjects = list({comment.subject for comment in approved_comments}) result.lecturers.append(lecturer_to_result) diff --git a/rating_api/schemas/models.py b/rating_api/schemas/models.py index de510a8..157b26d 100644 --- a/rating_api/schemas/models.py +++ b/rating_api/schemas/models.py @@ -17,6 +17,7 @@ class CommentGet(Base): mark_kindness: int mark_freebie: int mark_clarity: int + mark_general: float lecturer_id: int From 0812caff002f357bf44309580eb390badcf23a82 Mon Sep 17 00:00:00 2001 From: Timur Enikeev Date: Mon, 18 Nov 2024 01:25:24 -0500 Subject: [PATCH 6/7] nulls_last fix --- rating_api/routes/lecturer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rating_api/routes/lecturer.py b/rating_api/routes/lecturer.py index 58d627d..55512b5 100644 --- a/rating_api/routes/lecturer.py +++ b/rating_api/routes/lecturer.py @@ -3,7 +3,7 @@ from auth_lib.fastapi import UnionAuth from fastapi import APIRouter, Depends, Query from fastapi_sqlalchemy import db -from sqlalchemy import and_, func, nullslast +from sqlalchemy import and_, func, nulls_last from rating_api.exceptions import AlreadyExists, ObjectNotFound from rating_api.models import Comment, Lecturer, LecturerUserComment, ReviewStatus @@ -108,7 +108,7 @@ async def get_lecturers( .filter(Lecturer.search_by_subject(subject)) .filter(Lecturer.search_by_name(name)) .order_by( - nullslast(func.avg(getattr(Comment, order_by)).desc()) + nulls_last(func.avg(getattr(Comment, order_by)).desc()) if "mark" in order_by else getattr(Lecturer, order_by) ) From 64e807a0b5ca0dd1601d548e79780189e12bcb91 Mon Sep 17 00:00:00 2001 From: Timur Enikeev Date: Mon, 18 Nov 2024 04:10:31 -0500 Subject: [PATCH 7/7] Add sort_order option --- rating_api/models/db.py | 15 ++++++++++++++- rating_api/routes/lecturer.py | 11 ++++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/rating_api/models/db.py b/rating_api/models/db.py index 6564765..1c0e1b8 100644 --- a/rating_api/models/db.py +++ b/rating_api/models/db.py @@ -7,9 +7,10 @@ from sqlalchemy import UUID, Boolean, DateTime from sqlalchemy import Enum as DbEnum -from sqlalchemy import ForeignKey, Integer, String, and_, func, or_, true +from sqlalchemy import ForeignKey, Integer, String, UnaryExpression, and_, func, nulls_last, or_, true from sqlalchemy.ext.hybrid import hybrid_method, hybrid_property from sqlalchemy.orm import Mapped, mapped_column, relationship +from sqlalchemy.orm.attributes import InstrumentedAttribute from rating_api.settings import get_settings @@ -60,6 +61,18 @@ def search_by_subject(self, query: str) -> bool: response = and_(Comment.review_status == ReviewStatus.APPROVED, func.lower(Comment.subject).contains(query)) return response + @hybrid_method + def order_by_mark(self, query: str, asc_order: bool) -> UnaryExpression[float]: + return ( + nulls_last(func.avg(getattr(Comment, query))) + if asc_order + else nulls_last(func.avg(getattr(Comment, query)).desc()) + ) + + @hybrid_method + def order_by_name(self, query: str, asc_order: bool) -> UnaryExpression[str] | InstrumentedAttribute[str]: + return getattr(Lecturer, query) if asc_order else getattr(Lecturer, query).desc() + class Comment(BaseDbModel): uuid: Mapped[uuid.UUID] = mapped_column(UUID, primary_key=True, default=uuid.uuid4) diff --git a/rating_api/routes/lecturer.py b/rating_api/routes/lecturer.py index 55512b5..1bdd563 100644 --- a/rating_api/routes/lecturer.py +++ b/rating_api/routes/lecturer.py @@ -3,7 +3,7 @@ from auth_lib.fastapi import UnionAuth from fastapi import APIRouter, Depends, Query from fastapi_sqlalchemy import db -from sqlalchemy import and_, func, nulls_last +from sqlalchemy import and_ from rating_api.exceptions import AlreadyExists, ObjectNotFound from rating_api.models import Comment, Lecturer, LecturerUserComment, ReviewStatus @@ -80,6 +80,7 @@ async def get_lecturers( ), subject: str = Query(''), name: str = Query(''), + asc_order: bool = False, ) -> LecturerGetAll: """ `limit` - максимальное количество возвращаемых преподавателей @@ -100,6 +101,10 @@ async def get_lecturers( `name` Поле для ФИО. Если передано `name` - возвращает всех преподователей, для которых нашлись совпадения с переданной строкой + + `asc_order` + Если передано true, сортировать в порядке возрастания + Иначе - в порядке убывания """ lecturers_query = ( Lecturer.query(session=db.session) @@ -108,9 +113,9 @@ async def get_lecturers( .filter(Lecturer.search_by_subject(subject)) .filter(Lecturer.search_by_name(name)) .order_by( - nulls_last(func.avg(getattr(Comment, order_by)).desc()) + Lecturer.order_by_mark(order_by, asc_order) if "mark" in order_by - else getattr(Lecturer, order_by) + else Lecturer.order_by_name(order_by, asc_order) ) )