fix: the add_pagination_to_query now always returns the correct count (#6505)

Co-authored-by: Michael Genson <71845777+michael-genson@users.noreply.github.com>
This commit is contained in:
Aurelien
2025-12-22 17:39:35 +01:00
committed by GitHub
parent 6ed85d72d7
commit 7b32508201
2 changed files with 40 additions and 10 deletions

View File

@@ -364,14 +364,16 @@ class RepositoryGeneric[Schema: MealieModel, Model: SqlAlchemyBase]:
self.logger.error(e) self.logger.error(e)
raise HTTPException(status_code=400, detail=str(e)) from e raise HTTPException(status_code=400, detail=str(e)) from e
count_query = select(func.count()).select_from(query.subquery()) count_query = select(func.count()).select_from(query.order_by(None).distinct().subquery())
count = self.session.scalar(count_query) count = self.session.scalar(count_query)
if not count: if not count:
count = 0 count = 0
# interpret -1 as "get_all" # interpret -1 as "get_all"
limit: int | None = pagination.per_page
if pagination.per_page == -1: if pagination.per_page == -1:
pagination.per_page = count pagination.per_page = count
limit = None
try: try:
total_pages = ceil(count / pagination.per_page) total_pages = ceil(count / pagination.per_page)
@@ -387,7 +389,11 @@ class RepositoryGeneric[Schema: MealieModel, Model: SqlAlchemyBase]:
pagination.page = 1 pagination.page = 1
query = self.add_order_by_to_query(query, pagination) query = self.add_order_by_to_query(query, pagination)
return query.limit(pagination.per_page).offset((pagination.page - 1) * pagination.per_page), count, total_pages
if limit is not None:
query = query.limit(limit)
return query.offset((pagination.page - 1) * pagination.per_page), count, total_pages
def add_order_attr_to_query( def add_order_attr_to_query(
self, self,

View File

@@ -131,6 +131,29 @@ def test_pagination_response_and_metadata(unique_user: TestUser):
assert last_page_of_results.items[-1] == all_results.items[-1] assert last_page_of_results.items[-1] == all_results.items[-1]
def test_pagination_total_calculation(unique_user: TestUser):
db = unique_user.repos
unique_category_1, unused_category = (
db.categories.create(CategorySave(group_id=unique_user.group_id, name=random_string(10))) for _ in range(2)
)
recipe_1, recipe_2 = (
db.recipes.create(Recipe(user_id=unique_user.user_id, group_id=unique_user.group_id, name=random_string()))
for _ in range(2)
)
recipe_1.recipe_category = [unique_category_1]
db.recipes.update(recipe_1.slug, recipe_1)
db.recipes.update(recipe_2.slug, recipe_2)
query = PaginationQuery(page=1, per_page=64, query_filter=f"recipeCategory.name NOT IN [{unique_category_1.name}]")
result = db.recipes.page_all(query)
assert result.total == 1
query = PaginationQuery(page=1, per_page=64, query_filter=f"recipeCategory.name NOT IN [{unused_category.name}]")
result = db.recipes.page_all(query)
assert result.total == 2
def test_pagination_guides(unique_user: TestUser): def test_pagination_guides(unique_user: TestUser):
database = unique_user.repos database = unique_user.repos
group = database.groups.get_one(unique_user.group_id) group = database.groups.get_one(unique_user.group_id)
@@ -239,19 +262,19 @@ def test_pagination_filter_string_case_insensitive(
units_repo.delete(upper_unit.id) units_repo.delete(upper_unit.id)
def test_pagination_filter_null(unique_user: TestUser): def test_pagination_filter_null(unique_user_fn_scoped: TestUser):
database = unique_user.repos database = unique_user_fn_scoped.repos
recipe_not_made_1 = database.recipes.create( recipe_not_made_1 = database.recipes.create(
Recipe( Recipe(
user_id=unique_user.user_id, user_id=unique_user_fn_scoped.user_id,
group_id=unique_user.group_id, group_id=unique_user_fn_scoped.group_id,
name=random_string(), name=random_string(),
) )
) )
recipe_not_made_2 = database.recipes.create( recipe_not_made_2 = database.recipes.create(
Recipe( Recipe(
user_id=unique_user.user_id, user_id=unique_user_fn_scoped.user_id,
group_id=unique_user.group_id, group_id=unique_user_fn_scoped.group_id,
name=random_string(), name=random_string(),
) )
) )
@@ -259,8 +282,8 @@ def test_pagination_filter_null(unique_user: TestUser):
# give one recipe a last made date # give one recipe a last made date
recipe_made = database.recipes.create( recipe_made = database.recipes.create(
Recipe( Recipe(
user_id=unique_user.user_id, user_id=unique_user_fn_scoped.user_id,
group_id=unique_user.group_id, group_id=unique_user_fn_scoped.group_id,
name=random_string(), name=random_string(),
last_made=datetime.now(UTC), last_made=datetime.now(UTC),
) )
@@ -370,6 +393,7 @@ def test_pagination_filter_not_in_m2m(unique_user: TestUser):
db.recipes.update(recipe_2.slug, recipe_2) db.recipes.update(recipe_2.slug, recipe_2)
query = PaginationQuery(page=1, per_page=-1, query_filter=f"recipeCategory.name NOT IN [{unique_category_1.name}]") query = PaginationQuery(page=1, per_page=-1, query_filter=f"recipeCategory.name NOT IN [{unique_category_1.name}]")
recipe_results = db.recipes.page_all(query).items recipe_results = db.recipes.page_all(query).items
recipe_results_ids = {recipe.id for recipe in recipe_results} recipe_results_ids = {recipe.id for recipe in recipe_results}
assert recipe_1.id not in recipe_results_ids assert recipe_1.id not in recipe_results_ids