From 95e434a7501ebc1aae838e9f402259cd9328c9a0 Mon Sep 17 00:00:00 2001 From: Francisco Penedo Alvarez Date: Sat, 21 Mar 2026 18:53:26 +0100 Subject: [PATCH] Add sorting by rating --- src/hxbooks/library.py | 87 +++++++++++++++++++++++++----------------- tests/test_cli.py | 5 +++ 2 files changed, 58 insertions(+), 34 deletions(-) diff --git a/src/hxbooks/library.py b/src/hxbooks/library.py index 953b9f6..9f8f73e 100644 --- a/src/hxbooks/library.py +++ b/src/hxbooks/library.py @@ -9,7 +9,7 @@ from collections.abc import Sequence from datetime import date, datetime from typing import assert_never -from sqlalchemy import ColumnElement, and_, func, or_, select +from sqlalchemy import ColumnElement, Select, and_, func, or_, select from sqlalchemy.orm import InstrumentedAttribute, joinedload from hxbooks.search import IsOperatorValue, QueryParser, SortDirection, ValueT @@ -313,31 +313,9 @@ def search_books_advanced( sort_columns = [] for field_filter in parsed_query.field_filters: if field_filter.field == Field.SORT: - if ( - isinstance(field_filter.value, tuple) - and field_filter.value[0] == Field.READ_DATE - ): - # Special handling for sorting by read date - sort by latest reading end - # date - subq = ( - select( - Reading.book_id, - func.max(Reading.end_date).label("latest_read_date"), - ) - .where( - Reading.user.has(User.username == username), - Reading.end_date.isnot(None), - ) - .group_by(Reading.book_id) - .subquery() - ) - query = query.outerjoin(subq, Book.id == subq.c.book_id) - if field_filter.value[1] == SortDirection.ASC: - sort_columns.append(subq.c.latest_read_date.asc().nullslast()) - else: - sort_columns.append(subq.c.latest_read_date.desc().nullslast()) - else: - sort_columns.append(_build_sort_column(field_filter.value)) + query, sort_column = _build_sort_column(query, field_filter.value, username) + if sort_column is not None: + sort_columns.append(sort_column) else: condition = _build_field_condition(field_filter, username) @@ -449,7 +427,9 @@ def _build_field_condition( return condition -def _build_sort_column(value: ValueT) -> ColumnElement | None: +def _build_sort_column( + query: Select, value: ValueT, username: str | None = None +) -> tuple[Select, ColumnElement | None]: """Build a sort column for the 'sort' field.""" assert isinstance(value, tuple) and len(value) == 2 field, direction = value @@ -476,21 +456,60 @@ def _build_sort_column(value: ValueT) -> ColumnElement | None: column = Book.location_shelf case Field.OWNER: column = User.username - # These relationship-based fields are not supported here as they need subqueries - case Field.RATING | Field.READ_DATE: - return None + case Field.READ_DATE: + # Special handling for sorting by read date - sort by latest reading end + # date + subq = ( + select( + Reading.book_id, + func.max(Reading.end_date).label("latest_read_date"), + ) + .where( + Reading.user.has(User.username == username), + Reading.end_date.isnot(None), + ) + .group_by(Reading.book_id) + .subquery("latest_readings") + ) + query = query.outerjoin(subq, Book.id == subq.c.book_id) + column = subq.c.latest_read_date + case Field.RATING: + # Special handling for sorting by rating - sort by latest reading rating + subq = ( + select( + Reading.book_id, + Reading.rating.label("latest_rating"), + func + .row_number() + .over( + partition_by=Reading.book_id, + order_by=Reading.end_date.desc(), + ) + .label("rn"), + ) + .where( + Reading.user.has(User.username == username), + Reading.rating.isnot(None), + Reading.end_date.isnot(None), + ) + .subquery("latest_ratings") + ) + query = query.outerjoin( + subq, (Book.id == subq.c.book_id) & (subq.c.rn == 1) + ) + column = subq.c.latest_rating # These fields don't make sense to sort by case Field.AUTHOR | Field.GENRE | Field.IS | Field.SORT: - return None + return query, None case _: assert_never(field) if direction == SortDirection.ASC: - return column.asc().nullslast() + return query, column.asc().nullslast() elif direction == SortDirection.DESC: - return column.desc().nullslast() + return query, column.desc().nullslast() else: - return None + return query, None def _apply_operator( diff --git a/tests/test_cli.py b/tests/test_cli.py index 752fcfb..c466721 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -377,6 +377,11 @@ class TestBookSearchCommand: "", ["Dune", "The Fellowship", "The Hobbit", "Programming Book"], ), + ( + "sort:rating-desc", + "alice", + ["The Hobbit", "Dune", "Programming Book", "The Fellowship"], + ), ], ) def test_book_search_advanced_queries(