Skip to content

Commit 31dd624

Browse files
authored
fix: correct pagination display when DB-to-Pydantic conversion fails (#3238)
Fixes #2851 When items fail Pydantic validation during admin list views, the pagination 'Showing X - Y of Z' display was incorrect. The server adjusted total_items (Z) but the X-Y range was computed client-side using requested items, not actual rendered count. Changes: - Add optional page_items field to PaginationMeta schema - Update _adjust_pagination_for_conversion_failures() to set page_items - Update 6 admin partial endpoints to pass rendered item count - Update pagination_controls.html to use page_items when available - Update tests to match new function signature Example: 20 items requested, 2 fail conversion - Before: 'Showing 1 - 20 of 98 items' (18 rows displayed) - After: 'Showing 1 - 18 of 98 items' (18 rows displayed) Builds on PR #2846 which introduced _adjust_pagination_for_conversion_failures() Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com>
1 parent 2e42613 commit 31dd624

File tree

4 files changed

+30
-16
lines changed

4 files changed

+30
-16
lines changed

mcpgateway/admin.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -697,16 +697,18 @@ def _get_user_team_roles(db: Session, user_email: str) -> Dict[str, str]:
697697
return get_user_team_roles(db, user_email)
698698

699699

700-
def _adjust_pagination_for_conversion_failures(pagination: "PaginationMeta", failed_count: int) -> None:
700+
def _adjust_pagination_for_conversion_failures(pagination: "PaginationMeta", failed_count: int, rendered_count: int) -> None:
701701
"""Adjust pagination metadata to account for DB-to-Pydantic conversion failures.
702702

703703
When items on the current page fail to convert, the "Showing X of Y" display
704704
would otherwise count items that aren't actually displayed. This adjusts
705705
total_items and recomputes derived fields (total_pages, has_next, has_prev).
706+
Also sets page_items to the actual number of successfully rendered items.
706707

707708
Args:
708709
pagination: The PaginationMeta object to adjust (modified in-place).
709710
failed_count: Number of items that failed conversion on the current page.
711+
rendered_count: Number of items successfully converted and rendered on the current page.
710712
"""
711713
if failed_count > 0:
712714
pagination.total_items = max(0, pagination.total_items - failed_count)
@@ -715,6 +717,8 @@ def _adjust_pagination_for_conversion_failures(pagination: "PaginationMeta", fai
715717
# so the page number must match the displayed data.
716718
pagination.has_next = pagination.page < pagination.total_pages
717719
pagination.has_prev = pagination.page > 1
720+
# Always set page_items to reflect actual rendered count (even if failed_count == 0)
721+
pagination.page_items = rendered_count
718722

719723

720724
def _get_span_entity_performance(
@@ -2386,7 +2390,7 @@ async def admin_servers_partial_html(
23862390
except (ValidationError, ValueError, KeyError, TypeError, binascii.Error) as e:
23872391
failed_count += 1
23882392
LOGGER.exception(f"Failed to convert server {getattr(s, 'id', 'unknown')} ({getattr(s, 'name', 'unknown')}): {e}")
2389-
_adjust_pagination_for_conversion_failures(pagination, failed_count)
2393+
_adjust_pagination_for_conversion_failures(pagination, failed_count, len(servers_pydantic))
23902394
data = jsonable_encoder(servers_pydantic)
23912395

23922396
# End the read-only transaction before template rendering to avoid idle-in-transaction timeouts.
@@ -8032,7 +8036,7 @@ async def admin_tools_partial_html(
80328036
except (ValidationError, ValueError, KeyError, TypeError, binascii.Error) as e:
80338037
failed_count += 1
80348038
LOGGER.exception(f"Failed to convert tool {getattr(t, 'id', 'unknown')} ({getattr(t, 'name', 'unknown')}): {e}")
8035-
_adjust_pagination_for_conversion_failures(pagination, failed_count)
8039+
_adjust_pagination_for_conversion_failures(pagination, failed_count, len(tools_pydantic))
80368040

80378041
# Serialize tools
80388042
data = jsonable_encoder(tools_pydantic)
@@ -8606,7 +8610,7 @@ async def admin_prompts_partial_html(
86068610
except (ValidationError, ValueError, KeyError, TypeError, binascii.Error) as e:
86078611
failed_count += 1
86088612
LOGGER.exception(f"Failed to convert prompt {getattr(p, 'id', 'unknown')} ({getattr(p, 'name', 'unknown')}): {e}")
8609-
_adjust_pagination_for_conversion_failures(pagination, failed_count)
8613+
_adjust_pagination_for_conversion_failures(pagination, failed_count, len(prompts_pydantic))
86108614

86118615
data = jsonable_encoder(prompts_pydantic)
86128616

@@ -8811,7 +8815,7 @@ async def admin_gateways_partial_html(
88118815
except (ValidationError, ValueError, KeyError, TypeError, binascii.Error) as e:
88128816
failed_count += 1
88138817
LOGGER.exception(f"Failed to convert gateway {getattr(g, 'id', 'unknown')} ({getattr(g, 'name', 'unknown')}): {e}")
8814-
_adjust_pagination_for_conversion_failures(pagination, failed_count)
8818+
_adjust_pagination_for_conversion_failures(pagination, failed_count, len(gateways_pydantic))
88158819
data = jsonable_encoder(gateways_pydantic)
88168820

88178821
# End the read-only transaction before template rendering to avoid idle-in-transaction timeouts.
@@ -9391,7 +9395,7 @@ async def admin_resources_partial_html(
93919395
except (ValidationError, ValueError, KeyError, TypeError, binascii.Error) as e:
93929396
failed_count += 1
93939397
LOGGER.exception(f"Failed to convert resource {getattr(r, 'id', 'unknown')} ({getattr(r, 'name', 'unknown')}): {e}")
9394-
_adjust_pagination_for_conversion_failures(pagination, failed_count)
9398+
_adjust_pagination_for_conversion_failures(pagination, failed_count, len(resources_pydantic))
93959399

93969400
data = jsonable_encoder(resources_pydantic)
93979401

@@ -10273,7 +10277,7 @@ async def admin_a2a_partial_html(
1027310277
except (ValidationError, ValueError, KeyError, TypeError, binascii.Error) as e:
1027410278
failed_count += 1
1027510279
LOGGER.exception(f"Failed to convert a2a agent {getattr(a, 'id', 'unknown')} ({getattr(a, 'name', 'unknown')}): {e}")
10276-
_adjust_pagination_for_conversion_failures(pagination, failed_count)
10280+
_adjust_pagination_for_conversion_failures(pagination, failed_count, len(a2a_agents_pydantic))
1027710281
data = jsonable_encoder(a2a_agents_pydantic)
1027810282

1027910283
# End the read-only transaction before template rendering to avoid idle-in-transaction timeouts.

mcpgateway/schemas.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7292,6 +7292,7 @@ class PaginationMeta(BaseModel):
72927292
total_pages: int = Field(..., description="Total number of pages", ge=0)
72937293
has_next: bool = Field(..., description="Whether there is a next page")
72947294
has_prev: bool = Field(..., description="Whether there is a previous page")
7295+
page_items: Optional[int] = Field(None, description="Actual number of items on current page (after conversion failures)", ge=0)
72957296
next_cursor: Optional[str] = Field(None, description="Cursor for next page (cursor-based only)")
72967297
prev_cursor: Optional[str] = Field(None, description="Cursor for previous page (cursor-based only)")
72977298

mcpgateway/templates/pagination_controls.html

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
totalPages: {{ pagination.total_pages }},
2222
hasNext: {{ 'true' if pagination.has_next else 'false' }},
2323
hasPrev: {{ 'true' if pagination.has_prev else 'false' }},
24+
pageItems: {{ pagination.page_items if pagination.page_items is not none else 'null' }},
2425
targetSelector: '{{ hx_target|default('#tools-table') }}',
2526
swapStyle: '{{ hx_swap|default('innerHTML') }}',
2627
tableName: '{{ table_name|default('') }}',
@@ -185,7 +186,7 @@
185186
<!-- Page Info -->
186187
<div class="text-sm text-gray-700 dark:text-gray-300">
187188
<span
188-
x-text="totalItems === 0 ? 'No items found' : `Showing ${Math.min((currentPage - 1) * perPage + 1, totalItems)} - ${Math.min(currentPage * perPage, totalItems)} of ${totalItems.toLocaleString()} items`"
189+
x-text="totalItems === 0 ? 'No items found' : `Showing ${Math.min((currentPage - 1) * perPage + 1, totalItems)} - ${pageItems !== null ? Math.min((currentPage - 1) * perPage + pageItems, totalItems) : Math.min(currentPage * perPage, totalItems)} of ${totalItems.toLocaleString()} items`"
189190
></span>
190191
</div>
191192

tests/unit/mcpgateway/test_admin.py

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17472,58 +17472,66 @@ def _make_pagination(self, total_items: int = 100, page: int = 1, per_page: int
1747217472

1747317473
def test_decrements_total_items_by_failed_count(self):
1747417474
pagination = self._make_pagination(total_items=100)
17475-
_adjust_pagination_for_conversion_failures(pagination, failed_count=3)
17475+
_adjust_pagination_for_conversion_failures(pagination, failed_count=3, rendered_count=17)
1747617476
assert pagination.total_items == 97
1747717477
assert pagination.total_pages == 5
1747817478
assert pagination.has_next is True
1747917479
assert pagination.has_prev is False
17480+
assert pagination.page_items == 17
1748017481

1748117482
def test_zero_failures_leaves_total_unchanged(self):
1748217483
pagination = self._make_pagination(total_items=50)
17483-
_adjust_pagination_for_conversion_failures(pagination, failed_count=0)
17484+
_adjust_pagination_for_conversion_failures(pagination, failed_count=0, rendered_count=20)
1748417485
assert pagination.total_items == 50
17486+
assert pagination.page_items == 20
1748517487

1748617488
def test_floors_at_zero_when_failures_exceed_total(self):
1748717489
pagination = self._make_pagination(total_items=2)
17488-
_adjust_pagination_for_conversion_failures(pagination, failed_count=5)
17490+
_adjust_pagination_for_conversion_failures(pagination, failed_count=5, rendered_count=0)
1748917491
assert pagination.total_items == 0
1749017492
assert pagination.total_pages == 0
1749117493
assert pagination.has_next is False
1749217494
assert pagination.has_prev is False
17495+
assert pagination.page_items == 0
1749317496

1749417497
def test_exact_match_results_in_zero(self):
1749517498
pagination = self._make_pagination(total_items=10)
17496-
_adjust_pagination_for_conversion_failures(pagination, failed_count=10)
17499+
_adjust_pagination_for_conversion_failures(pagination, failed_count=10, rendered_count=0)
1749717500
assert pagination.total_items == 0
1749817501
assert pagination.total_pages == 0
17502+
assert pagination.page_items == 0
1749917503

1750017504
def test_total_items_already_zero(self):
1750117505
pagination = self._make_pagination(total_items=0)
17502-
_adjust_pagination_for_conversion_failures(pagination, failed_count=3)
17506+
_adjust_pagination_for_conversion_failures(pagination, failed_count=3, rendered_count=0)
1750317507
assert pagination.total_items == 0
17508+
assert pagination.page_items == 0
1750417509

1750517510
def test_single_failure(self):
1750617511
pagination = self._make_pagination(total_items=1)
17507-
_adjust_pagination_for_conversion_failures(pagination, failed_count=1)
17512+
_adjust_pagination_for_conversion_failures(pagination, failed_count=1, rendered_count=0)
1750817513
assert pagination.total_items == 0
1750917514
assert pagination.total_pages == 0
17515+
assert pagination.page_items == 0
1751017516

1751117517
def test_recomputes_has_next_on_boundary(self):
1751217518
"""When failures reduce total_pages, has_next should become False."""
1751317519
pagination = self._make_pagination(total_items=21, page=1, per_page=20)
1751417520
assert pagination.has_next is True
17515-
_adjust_pagination_for_conversion_failures(pagination, failed_count=2)
17521+
_adjust_pagination_for_conversion_failures(pagination, failed_count=2, rendered_count=18)
1751617522
assert pagination.total_items == 19
1751717523
assert pagination.total_pages == 1
1751817524
assert pagination.has_next is False
17525+
assert pagination.page_items == 18
1751917526

1752017527
def test_page_not_clamped_when_total_pages_shrinks(self):
1752117528
"""Page is NOT clamped because data was already fetched for this page."""
1752217529
pagination = self._make_pagination(total_items=41, page=3, per_page=20)
17523-
_adjust_pagination_for_conversion_failures(pagination, failed_count=2)
17530+
_adjust_pagination_for_conversion_failures(pagination, failed_count=2, rendered_count=18)
1752417531
assert pagination.total_items == 39
1752517532
assert pagination.total_pages == 2
1752617533
assert pagination.page == 3 # stays at queried page, not clamped
17534+
assert pagination.page_items == 18
1752717535
assert pagination.has_next is False
1752817536
assert pagination.has_prev is True
1752917537

0 commit comments

Comments
 (0)