From 434bfed97788548aa0be2f7d05be3bd21a9867a3 Mon Sep 17 00:00:00 2001 From: Jaspreet Singh <6873201+tagpro@users.noreply.github.com> Date: Sat, 11 Jul 2026 16:23:52 +1000 Subject: [PATCH] fix(notifications): paginate and index the notification list endpoints The notification tab loaded slowly because both list endpoints returned the entire notification table. GET /notification and GET /notification/unread ran `select(Notification).order_by(timestamp desc)` with no limit, so every page load fetched every row, pydantic-validated each one, serialised the lot to JSON and shipped it to the browser. On the instance this was found on the table had ~437,000 rows. The table also had no index beyond the primary key on `id`, so the ORDER BY sorted the whole table on every load and the unread filter was a full scan. Backend: - Both list endpoints now take `limit` (default 50, max 200) and `offset` (>= 0), threaded through the service into `.limit()/.offset()` in the SQL. Ordering stays timestamp descending. The number of matching notifications is returned in the `X-Total-Count` response header (exposed via CORS) so the UI can paginate. - GET /notification takes an optional `read` filter, so the UI can page through read notifications rather than fetching everything and filtering client-side. - New migration indexes the table for these queries: an index on `timestamp` descending for the ORDER BY, and a partial index for the unread query. - New PATCH /notification/read marks every unread notification read in a single statement. The UI used to do this by looping a PATCH over every notification it held, which only worked because it held all of them. Frontend: - The notifications page now loads a page of 50 with "load more", and does not fetch read notifications at all until that section is expanded. Tests: - Repository tests covering the limit cap, offset, timestamp-descending ordering, the unread query, and the bulk mark-as-read, seeded with more rows than the page size so pagination is actually exercised. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019wrZPxJDip5x3mzjWnpwos --- ...2_index_notification_timestamp_and_read.py | 44 +++ media_manager/main.py | 1 + media_manager/notification/models.py | 16 +- media_manager/notification/repository.py | 59 +++- media_manager/notification/router.py | 66 ++++- media_manager/notification/service.py | 24 +- pyproject.toml | 11 + tests/__init__.py | 0 tests/conftest.py | 43 +++ tests/test_notification_repository.py | 257 ++++++++++++++++++ uv.lock | 31 +++ web/src/lib/api/api.d.ts | 85 +++++- .../dashboard/notifications/+page.svelte | 228 ++++++++++++---- 13 files changed, 795 insertions(+), 70 deletions(-) create mode 100644 alembic/versions/b3d51c7fa9e2_index_notification_timestamp_and_read.py create mode 100644 tests/__init__.py create mode 100644 tests/conftest.py create mode 100644 tests/test_notification_repository.py diff --git a/alembic/versions/b3d51c7fa9e2_index_notification_timestamp_and_read.py b/alembic/versions/b3d51c7fa9e2_index_notification_timestamp_and_read.py new file mode 100644 index 00000000..af5808e9 --- /dev/null +++ b/alembic/versions/b3d51c7fa9e2_index_notification_timestamp_and_read.py @@ -0,0 +1,44 @@ +"""index notification timestamp and read + +Revision ID: b3d51c7fa9e2 +Revises: e60ae827ed98 +Create Date: 2026-07-11 10:12:38.114905 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision: str = 'b3d51c7fa9e2' +down_revision: Union[str, None] = 'e60ae827ed98' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + # The notification list is ordered by timestamp descending and paginated, + # so the index matches that order to keep a page from sorting the table. + op.create_index( + 'ix_notification_timestamp_desc', + 'notification', + [sa.text('timestamp DESC')], + unique=False, + ) + # Unread notifications are a small fraction of the table, so a partial + # index keeps the unread query off a full table scan. + op.create_index( + 'ix_notification_unread_timestamp_desc', + 'notification', + [sa.text('timestamp DESC')], + unique=False, + postgresql_where=sa.text('read = false'), + ) + + +def downgrade() -> None: + """Downgrade schema.""" + op.drop_index('ix_notification_unread_timestamp_desc', table_name='notification') + op.drop_index('ix_notification_timestamp_desc', table_name='notification') diff --git a/media_manager/main.py b/media_manager/main.py index a36b4be9..f8a00300 100644 --- a/media_manager/main.py +++ b/media_manager/main.py @@ -141,6 +141,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator: allow_origins=origins, allow_credentials=True, allow_methods=["GET", "PUT", "POST", "DELETE", "PATCH", "HEAD", "OPTIONS"], + expose_headers=["X-Total-Count"], ) app.add_middleware(CorrelationIdMiddleware, header_name="X-Correlation-ID") api_app = APIRouter(prefix="/api/v1") diff --git a/media_manager/notification/models.py b/media_manager/notification/models.py index f7772867..fabd3a5b 100644 --- a/media_manager/notification/models.py +++ b/media_manager/notification/models.py @@ -1,6 +1,6 @@ from uuid import UUID -from sqlalchemy import DateTime +from sqlalchemy import DateTime, Index, text from sqlalchemy.orm import Mapped, mapped_column from media_manager.database import Base @@ -13,3 +13,17 @@ class Notification(Base): message: Mapped[str] read: Mapped[bool] timestamp = mapped_column(DateTime, nullable=False) + + __table_args__ = ( + # Notifications are always listed newest first and are paginated, so the + # index matches that order to keep a page from sorting the whole table. + Index("ix_notification_timestamp_desc", text("timestamp DESC")), + # Unread notifications are a small fraction of the table, so a partial + # index keeps the unread query off a full table scan. + Index( + "ix_notification_unread_timestamp_desc", + text("timestamp DESC"), + postgresql_where=text("read = false"), + sqlite_where=text("read = 0"), + ), + ) diff --git a/media_manager/notification/repository.py b/media_manager/notification/repository.py index 1ab9b7c9..696d5405 100644 --- a/media_manager/notification/repository.py +++ b/media_manager/notification/repository.py @@ -1,12 +1,13 @@ import logging -from sqlalchemy import delete, select, update +from sqlalchemy import delete, func, select, update from sqlalchemy.exc import ( IntegrityError, SQLAlchemyError, ) from sqlalchemy.ext.asyncio import AsyncSession -from sqlalchemy.sql.expression import false +from sqlalchemy.sql import Select +from sqlalchemy.sql.expression import false, true from media_manager.exceptions import ConflictError, NotFoundError from media_manager.notification.models import Notification @@ -20,6 +21,12 @@ log = logging.getLogger(__name__) +def _filtered_by_read(stmt: Select, read: bool | None) -> Select: + if read is None: + return stmt + return stmt.where(Notification.read == (true() if read else false())) + + class NotificationRepository: def __init__(self, db: AsyncSession) -> None: self.db = db @@ -33,12 +40,15 @@ async def get_notification(self, nid: NotificationId) -> NotificationSchema: return NotificationSchema.model_validate(result) - async def get_unread_notifications(self) -> list[NotificationSchema]: + async def get_unread_notifications( + self, limit: int, offset: int = 0 + ) -> list[NotificationSchema]: try: stmt = ( - select(Notification) - .where(Notification.read == false()) + _filtered_by_read(select(Notification), read=False) .order_by(Notification.timestamp.desc()) + .limit(limit) + .offset(offset) ) results = (await self.db.execute(stmt)).scalars().all() return [ @@ -49,9 +59,21 @@ async def get_unread_notifications(self) -> list[NotificationSchema]: log.exception("Database error while retrieving unread notifications") raise - async def get_all_notifications(self) -> list[NotificationSchema]: + async def get_all_notifications( + self, limit: int, offset: int = 0, read: bool | None = None + ) -> list[NotificationSchema]: + """Return one page of notifications, newest first. + + ``read`` optionally restricts the page to read or unread notifications; + ``None`` returns both. + """ try: - stmt = select(Notification).order_by(Notification.timestamp.desc()) + stmt = ( + _filtered_by_read(select(Notification), read=read) + .order_by(Notification.timestamp.desc()) + .limit(limit) + .offset(offset) + ) results = (await self.db.execute(stmt)).scalars().all() return [ NotificationSchema.model_validate(notification) @@ -61,6 +83,17 @@ async def get_all_notifications(self) -> list[NotificationSchema]: log.exception("Database error while retrieving notifications") raise + async def count_notifications(self, read: bool | None = None) -> int: + """Count the notifications a matching page would be taken from.""" + try: + stmt = _filtered_by_read( + select(func.count()).select_from(Notification), read=read + ) + return (await self.db.execute(stmt)).scalar_one() + except SQLAlchemyError: + log.exception("Database error while counting notifications") + raise + async def save_notification(self, notification: NotificationSchema) -> None: try: self.db.add( @@ -91,6 +124,18 @@ async def mark_notification_as_unread(self, nid: NotificationId) -> None: await self.db.execute(stmt) return + async def mark_all_notifications_as_read(self) -> None: + """Mark every unread notification as read in a single statement. + + The UI only holds a page of notifications now, so it can no longer mark + them all read by looping over the ones it has. + """ + stmt = ( + update(Notification).where(Notification.read == false()).values(read=True) + ) + await self.db.execute(stmt) + return + async def delete_notification(self, nid: NotificationId) -> None: stmt = delete(Notification).where(Notification.id == nid) result = await self.db.execute(stmt) diff --git a/media_manager/notification/router.py b/media_manager/notification/router.py index b40640d4..03202d06 100644 --- a/media_manager/notification/router.py +++ b/media_manager/notification/router.py @@ -1,4 +1,6 @@ -from fastapi import APIRouter, Depends, status +from typing import Annotated + +from fastapi import APIRouter, Depends, Query, Response, status from media_manager.auth.users import current_active_user from media_manager.notification.dependencies import notification_service_dep @@ -6,6 +8,19 @@ router = APIRouter() +DEFAULT_PAGE_SIZE = 50 +MAX_PAGE_SIZE = 200 + +TOTAL_COUNT_HEADER = "X-Total-Count" + +limit_dep = Annotated[ + int, + Query( + ge=1, le=MAX_PAGE_SIZE, description="Maximum number of notifications to return" + ), +] +offset_dep = Annotated[int, Query(ge=0, description="Number of notifications to skip")] + # -------------------------------- # GET NOTIFICATIONS @@ -18,11 +33,25 @@ ) async def get_all_notifications( notification_service: notification_service_dep, + response: Response, + limit: limit_dep = DEFAULT_PAGE_SIZE, + offset: offset_dep = 0, + read: Annotated[ + bool | None, + Query(description="Only return read (true) or unread (false) notifications"), + ] = None, ) -> list[Notification]: """ - Get all notifications. + Get a page of notifications, newest first. + + The total number of matching notifications is returned in the + `X-Total-Count` response header. """ - return await notification_service.get_all_notifications() + total = await notification_service.count_notifications(read=read) + response.headers[TOTAL_COUNT_HEADER] = str(total) + return await notification_service.get_all_notifications( + limit=limit, offset=offset, read=read + ) @router.get( @@ -31,11 +60,21 @@ async def get_all_notifications( ) async def get_unread_notifications( notification_service: notification_service_dep, + response: Response, + limit: limit_dep = DEFAULT_PAGE_SIZE, + offset: offset_dep = 0, ) -> list[Notification]: """ - Get all unread notifications. + Get a page of unread notifications, newest first. + + The total number of unread notifications is returned in the `X-Total-Count` + response header. """ - return await notification_service.get_unread_notifications() + total = await notification_service.count_notifications(read=False) + response.headers[TOTAL_COUNT_HEADER] = str(total) + return await notification_service.get_unread_notifications( + limit=limit, offset=offset + ) @router.get( @@ -59,6 +98,23 @@ async def get_notification( # -------------------------------- +@router.patch( + "/read", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(current_active_user)], +) +async def mark_all_notifications_as_read( + notification_service: notification_service_dep, +) -> None: + """ + Mark every unread notification as read. + + The client only holds a page of notifications, so it cannot mark them all + read by iterating over the ones it has. + """ + await notification_service.mark_all_notifications_as_read() + + @router.patch( "/{notification_id}/read", status_code=status.HTTP_204_NO_CONTENT, diff --git a/media_manager/notification/service.py b/media_manager/notification/service.py index ee59af01..980fc6be 100644 --- a/media_manager/notification/service.py +++ b/media_manager/notification/service.py @@ -14,11 +14,22 @@ def __init__( async def get_notification(self, nid: NotificationId) -> Notification: return await self.notification_repository.get_notification(nid=nid) - async def get_unread_notifications(self) -> list[Notification]: - return await self.notification_repository.get_unread_notifications() - - async def get_all_notifications(self) -> list[Notification]: - return await self.notification_repository.get_all_notifications() + async def get_unread_notifications( + self, limit: int, offset: int = 0 + ) -> list[Notification]: + return await self.notification_repository.get_unread_notifications( + limit=limit, offset=offset + ) + + async def get_all_notifications( + self, limit: int, offset: int = 0, read: bool | None = None + ) -> list[Notification]: + return await self.notification_repository.get_all_notifications( + limit=limit, offset=offset, read=read + ) + + async def count_notifications(self, read: bool | None = None) -> int: + return await self.notification_repository.count_notifications(read=read) async def save_notification(self, notification: Notification) -> None: return await self.notification_repository.save_notification(notification) @@ -29,6 +40,9 @@ async def mark_notification_as_read(self, nid: NotificationId) -> None: async def mark_notification_as_unread(self, nid: NotificationId) -> None: return await self.notification_repository.mark_notification_as_unread(nid=nid) + async def mark_all_notifications_as_read(self) -> None: + return await self.notification_repository.mark_all_notifications_as_read() + async def delete_notification(self, nid: NotificationId) -> None: return await self.notification_repository.delete_notification(nid=nid) diff --git a/pyproject.toml b/pyproject.toml index 3775d049..fa31b71a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,6 +44,17 @@ dev = [ "ruff", "ty>=0.0.33", ] +test = [ + "pytest>=8.4.0", + "pytest-asyncio>=1.0.0", + "aiosqlite>=0.21.0", +] + +[tool.pytest.ini_options] +asyncio_mode = "auto" +asyncio_default_fixture_loop_scope = "function" +testpaths = ["tests"] +pythonpath = ["."] [tool.setuptools.packages.find] include = ["media_manager*"] diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 00000000..59fb1196 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,43 @@ +"""Shared fixtures for the repository test-suite. + +The tests run against an in-memory SQLite database driven by ``aiosqlite``, so +the suite needs no running PostgreSQL. Note that the indexer models are +deliberately not imported: they use a PostgreSQL-only ARRAY column. +""" + +from collections.abc import AsyncIterator + +import pytest +from sqlalchemy.ext.asyncio import ( + AsyncEngine, + AsyncSession, + async_sessionmaker, + create_async_engine, +) +from sqlalchemy.pool import StaticPool + +from media_manager.database import Base +from media_manager.notification import models as notification_models + +# Importing the model modules is what registers their tables on +# ``Base.metadata``; binding them here keeps the imports from looking unused. +_MODEL_MODULES = (notification_models,) + + +@pytest.fixture +async def engine() -> AsyncIterator[AsyncEngine]: + engine = create_async_engine( + "sqlite+aiosqlite://", + poolclass=StaticPool, + connect_args={"check_same_thread": False}, + ) + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + yield engine + await engine.dispose() + + +@pytest.fixture +def session_factory(engine: AsyncEngine) -> async_sessionmaker[AsyncSession]: + """Mirrors the production sessionmaker, notably ``expire_on_commit=False``.""" + return async_sessionmaker(engine, expire_on_commit=False) diff --git a/tests/test_notification_repository.py b/tests/test_notification_repository.py new file mode 100644 index 00000000..21d77158 --- /dev/null +++ b/tests/test_notification_repository.py @@ -0,0 +1,257 @@ +"""Tests for the pagination of the notification repository. + +The notification list endpoints used to select the *entire* notification table +(a live instance had ~437k rows), sort it, validate every row and ship the lot +to the browser. The repository now takes a ``limit``/``offset`` page, so these +tests pin the page down: it is bounded, it skips correctly, it stays ordered +newest first, and the unread page never leaks read notifications. + +Every assertion is made against a page smaller than the seeded row count, so a +regression to an unbounded query fails the suite rather than passing by luck. +""" + +import uuid +from datetime import datetime, timedelta + +import pytest +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from media_manager.notification.repository import NotificationRepository +from media_manager.notification.schemas import Notification as NotificationSchema +from media_manager.notification.schemas import NotificationId + +# Deliberately larger than any ``limit`` used below, so pagination is exercised. +SEEDED_NOTIFICATIONS = 120 + +BASE_TIME = datetime(2026, 1, 1, 12, 0, 0) + + +def build_notification(index: int, *, read: bool) -> NotificationSchema: + """Build notification ``index``, one minute newer than notification ``index - 1``.""" + return NotificationSchema( + id=NotificationId(uuid.uuid4()), + message=f"Notification {index}", + read=read, + timestamp=BASE_TIME + timedelta(minutes=index), + ) + + +@pytest.fixture +async def seeded_notifications( + session_factory: async_sessionmaker[AsyncSession], +) -> list[NotificationSchema]: + """Seed alternating read/unread notifications, returned newest first. + + Inserted in oldest-first order so that a query which forgets to order by + timestamp cannot accidentally return the expected order. + """ + notifications = [ + build_notification(index, read=index % 2 == 0) + for index in range(SEEDED_NOTIFICATIONS) + ] + async with session_factory() as session: + repository = NotificationRepository(session) + for notification in notifications: + await repository.save_notification(notification) + return sorted(notifications, key=lambda n: n.timestamp, reverse=True) + + +def ids(notifications: list[NotificationSchema]) -> list[uuid.UUID]: + return [notification.id for notification in notifications] + + +class TestGetAllNotifications: + async def test_limit_caps_the_number_returned( + self, + session_factory: async_sessionmaker[AsyncSession], + seeded_notifications: list[NotificationSchema], + ) -> None: + async with session_factory() as session: + page = await NotificationRepository(session).get_all_notifications(limit=50) + + assert len(page) == 50 + assert ids(page) == ids(seeded_notifications[:50]) + + async def test_offset_skips_the_preceding_notifications( + self, + session_factory: async_sessionmaker[AsyncSession], + seeded_notifications: list[NotificationSchema], + ) -> None: + async with session_factory() as session: + page = await NotificationRepository(session).get_all_notifications( + limit=50, offset=50 + ) + + assert ids(page) == ids(seeded_notifications[50:100]) + + async def test_consecutive_pages_cover_every_notification_exactly_once( + self, + session_factory: async_sessionmaker[AsyncSession], + seeded_notifications: list[NotificationSchema], + ) -> None: + collected: list[NotificationSchema] = [] + async with session_factory() as session: + repository = NotificationRepository(session) + for offset in range(0, SEEDED_NOTIFICATIONS, 50): + collected += await repository.get_all_notifications( + limit=50, offset=offset + ) + + assert ids(collected) == ids(seeded_notifications) + + async def test_offset_past_the_end_returns_an_empty_page( + self, + session_factory: async_sessionmaker[AsyncSession], + seeded_notifications: list[NotificationSchema], + ) -> None: + async with session_factory() as session: + page = await NotificationRepository(session).get_all_notifications( + limit=50, offset=SEEDED_NOTIFICATIONS + ) + + assert page == [] + + async def test_page_is_ordered_by_timestamp_descending( + self, + session_factory: async_sessionmaker[AsyncSession], + seeded_notifications: list[NotificationSchema], + ) -> None: + async with session_factory() as session: + page = await NotificationRepository(session).get_all_notifications(limit=50) + + timestamps = [notification.timestamp for notification in page] + assert timestamps == sorted(timestamps, reverse=True) + # The newest seeded notification has to be the first row of the first page. + assert page[0].timestamp == seeded_notifications[0].timestamp + + async def test_read_filter_returns_only_read_notifications( + self, + session_factory: async_sessionmaker[AsyncSession], + seeded_notifications: list[NotificationSchema], + ) -> None: + expected = [n for n in seeded_notifications if n.read] + async with session_factory() as session: + page = await NotificationRepository(session).get_all_notifications( + limit=20, offset=10, read=True + ) + + assert all(notification.read for notification in page) + assert ids(page) == ids(expected[10:30]) + + +class TestGetUnreadNotifications: + async def test_returns_only_unread_notifications( + self, + session_factory: async_sessionmaker[AsyncSession], + seeded_notifications: list[NotificationSchema], + ) -> None: + async with session_factory() as session: + page = await NotificationRepository(session).get_unread_notifications( + limit=50 + ) + + assert page != [] + assert not any(notification.read for notification in page) + + async def test_limit_caps_the_number_returned( + self, + session_factory: async_sessionmaker[AsyncSession], + seeded_notifications: list[NotificationSchema], + ) -> None: + unread = [n for n in seeded_notifications if not n.read] + assert len(unread) > 20, "the seed must exceed the limit under test" + + async with session_factory() as session: + page = await NotificationRepository(session).get_unread_notifications( + limit=20 + ) + + assert len(page) == 20 + assert ids(page) == ids(unread[:20]) + + async def test_offset_skips_the_preceding_unread_notifications( + self, + session_factory: async_sessionmaker[AsyncSession], + seeded_notifications: list[NotificationSchema], + ) -> None: + unread = [n for n in seeded_notifications if not n.read] + async with session_factory() as session: + page = await NotificationRepository(session).get_unread_notifications( + limit=20, offset=20 + ) + + assert ids(page) == ids(unread[20:40]) + + async def test_page_is_ordered_by_timestamp_descending( + self, + session_factory: async_sessionmaker[AsyncSession], + seeded_notifications: list[NotificationSchema], + ) -> None: + async with session_factory() as session: + page = await NotificationRepository(session).get_unread_notifications( + limit=50 + ) + + timestamps = [notification.timestamp for notification in page] + assert timestamps == sorted(timestamps, reverse=True) + + +class TestMarkAllNotificationsAsRead: + async def test_marks_every_unread_notification_read( + self, + session_factory: async_sessionmaker[AsyncSession], + seeded_notifications: list[NotificationSchema], + ) -> None: + """A single statement has to reach past the page the client holds.""" + async with session_factory() as session: + repository = NotificationRepository(session) + await repository.mark_all_notifications_as_read() + await session.commit() + + async with session_factory() as session: + repository = NotificationRepository(session) + unread = await repository.count_notifications(read=False) + read = await repository.count_notifications(read=True) + + assert unread == 0 + assert read == SEEDED_NOTIFICATIONS + + +class TestCountNotifications: + async def test_counts_every_notification_by_default( + self, + session_factory: async_sessionmaker[AsyncSession], + seeded_notifications: list[NotificationSchema], + ) -> None: + async with session_factory() as session: + total = await NotificationRepository(session).count_notifications() + + assert total == SEEDED_NOTIFICATIONS + + async def test_counts_unread_and_read_separately( + self, + session_factory: async_sessionmaker[AsyncSession], + seeded_notifications: list[NotificationSchema], + ) -> None: + async with session_factory() as session: + repository = NotificationRepository(session) + unread = await repository.count_notifications(read=False) + read = await repository.count_notifications(read=True) + + assert unread == len([n for n in seeded_notifications if not n.read]) + assert read == len([n for n in seeded_notifications if n.read]) + assert unread + read == SEEDED_NOTIFICATIONS + + async def test_count_is_independent_of_the_page_size( + self, + session_factory: async_sessionmaker[AsyncSession], + seeded_notifications: list[NotificationSchema], + ) -> None: + """The UI paginates on this count, so it must describe the whole table.""" + async with session_factory() as session: + repository = NotificationRepository(session) + page = await repository.get_all_notifications(limit=10) + total = await repository.count_notifications() + + assert len(page) == 10 + assert total == SEEDED_NOTIFICATIONS diff --git a/uv.lock b/uv.lock index 8a6c2832..046dfbe4 100644 --- a/uv.lock +++ b/uv.lock @@ -95,6 +95,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/fb/76/641ae371508676492379f16e2fa48f4e2c11741bd63c48be4b12a6b09cba/aiosignal-1.4.0-py3-none-any.whl", hash = "sha256:053243f8b92b990551949e63930a839ff0cf0b0ebbe0597b0f3fb19e1a0fe82e", size = 7490, upload-time = "2025-07-03T22:54:42.156Z" }, ] +[[package]] +name = "aiosqlite" +version = "0.22.1" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/4e/8a/64761f4005f17809769d23e518d915db74e6310474e733e3593cfc854ef1/aiosqlite-0.22.1.tar.gz", hash = "sha256:043e0bd78d32888c0a9ca90fc788b38796843360c855a7262a532813133a0650", size = 14821, upload-time = "2025-12-23T19:25:43.997Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/00/b7/e3bf5133d697a08128598c8d0abc5e16377b51465a33756de24fa7dee953/aiosqlite-0.22.1-py3-none-any.whl", hash = "sha256:21c002eb13823fad740196c5a2e9d8e62f6243bd9e7e4a1f87fb5e44ecb4fceb", size = 17405, upload-time = "2025-12-23T19:25:42.139Z" }, +] + [[package]] name = "alembic" version = "1.18.4" @@ -1068,6 +1077,11 @@ dev = [ { name = "ruff" }, { name = "ty" }, ] +test = [ + { name = "aiosqlite" }, + { name = "pytest" }, + { name = "pytest-asyncio" }, +] [package.metadata] requires-dist = [ @@ -1111,6 +1125,11 @@ dev = [ { name = "ruff" }, { name = "ty", specifier = ">=0.0.33" }, ] +test = [ + { name = "aiosqlite", specifier = ">=0.21.0" }, + { name = "pytest", specifier = ">=8.4.0" }, + { name = "pytest-asyncio", specifier = ">=1.0.0" }, +] [[package]] name = "multidict" @@ -1619,6 +1638,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/3b/ab/b3226f0bd7cdcf710fbede2b3548584366da3b19b5021e74f5bde2a8fa3f/pytest-9.0.2-py3-none-any.whl", hash = "sha256:711ffd45bf766d5264d487b917733b453d917afd2b0ad65223959f59089f875b", size = 374801, upload-time = "2025-12-06T21:30:49.154Z" }, ] +[[package]] +name = "pytest-asyncio" +version = "1.4.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pytest" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/43/7c/d36d04db312ecf4298932ef77e6e4a9e8ad017906e24e34f0b0c361a2473/pytest_asyncio-1.4.0.tar.gz", hash = "sha256:c6c0d2259945122819f171a32ecea2c349ead889ee28176caaf492143424be42", size = 58514, upload-time = "2026-05-26T09:56:04.083Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/03/e2/08a497ef684b88559c9cc5f4ad53a37e7b99e727094a86d6ea32536d5d3c/pytest_asyncio-1.4.0-py3-none-any.whl", hash = "sha256:933ca923a23075a87fb7070c0ec272a6848489824d887c85c812670932835aa1", size = 16930, upload-time = "2026-05-26T09:56:02.576Z" }, +] + [[package]] name = "python-dotenv" version = "1.2.2" diff --git a/web/src/lib/api/api.d.ts b/web/src/lib/api/api.d.ts index 08e74f2c..9c8605a0 100644 --- a/web/src/lib/api/api.d.ts +++ b/web/src/lib/api/api.d.ts @@ -942,7 +942,10 @@ export interface paths { }; /** * Get All Notifications - * @description Get all notifications. + * @description Get a page of notifications, newest first. + * + * The total number of matching notifications is returned in the + * `X-Total-Count` response header. */ get: operations['get_all_notifications_api_v1_notification_get']; put?: never; @@ -962,7 +965,10 @@ export interface paths { }; /** * Get Unread Notifications - * @description Get all unread notifications. + * @description Get a page of unread notifications, newest first. + * + * The total number of unread notifications is returned in the `X-Total-Count` + * response header. */ get: operations['get_unread_notifications_api_v1_notification_unread_get']; put?: never; @@ -997,6 +1003,29 @@ export interface paths { patch?: never; trace?: never; }; + '/api/v1/notification/read': { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + /** + * Mark All Notifications As Read + * @description Mark every unread notification as read. + * + * The client only holds a page of notifications, so it cannot mark them all + * read by iterating over the ones it has. + */ + patch: operations['mark_all_notifications_as_read_api_v1_notification_read_patch']; + trace?: never; + }; '/api/v1/notification/{notification_id}/read': { parameters: { query?: never; @@ -3694,7 +3723,14 @@ export interface operations { }; get_all_notifications_api_v1_notification_get: { parameters: { - query?: never; + query?: { + /** @description Maximum number of notifications to return */ + limit?: number; + /** @description Number of notifications to skip */ + offset?: number; + /** @description Only return read (true) or unread (false) notifications */ + read?: boolean | null; + }; header?: never; path?: never; cookie?: never; @@ -3710,11 +3746,25 @@ export interface operations { 'application/json': components['schemas']['Notification'][]; }; }; + /** @description Validation Error */ + 422: { + headers: { + [name: string]: unknown; + }; + content: { + 'application/json': components['schemas']['HTTPValidationError']; + }; + }; }; }; get_unread_notifications_api_v1_notification_unread_get: { parameters: { - query?: never; + query?: { + /** @description Maximum number of notifications to return */ + limit?: number; + /** @description Number of notifications to skip */ + offset?: number; + }; header?: never; path?: never; cookie?: never; @@ -3730,6 +3780,15 @@ export interface operations { 'application/json': components['schemas']['Notification'][]; }; }; + /** @description Validation Error */ + 422: { + headers: { + [name: string]: unknown; + }; + content: { + 'application/json': components['schemas']['HTTPValidationError']; + }; + }; }; }; get_notification_api_v1_notification__notification_id__get: { @@ -3806,6 +3865,24 @@ export interface operations { }; }; }; + mark_all_notifications_as_read_api_v1_notification_read_patch: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Successful Response */ + 204: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; mark_notification_as_read_api_v1_notification__notification_id__read_patch: { parameters: { query?: never; diff --git a/web/src/routes/dashboard/notifications/+page.svelte b/web/src/routes/dashboard/notifications/+page.svelte index 1f92d6d9..83b12b67 100644 --- a/web/src/routes/dashboard/notifications/+page.svelte +++ b/web/src/routes/dashboard/notifications/+page.svelte @@ -9,33 +9,117 @@ import type { Notification } from '$lib/api/api'; import { resolve } from '$app/paths'; + // The notification table grows without bound (a live instance had ~437k rows), + // so the page only ever holds the pages it has explicitly loaded. + const PAGE_SIZE = 50; + let unreadNotifications: Notification[] = []; let readNotifications: Notification[] = []; + // Totals come from the X-Total-Count header, so "load more" knows when to stop. + let unreadTotal = 0; + let readTotal = 0; let loading = true; + let loadingMoreUnread = false; + let loadingMoreRead = false; let showRead = false; let markingAllAsRead = false; - async function fetchNotifications() { - loading = true; - const unread = await client.GET('/api/v1/notification/unread'); - const all = await client.GET('/api/v1/notification'); - unreadNotifications = unread.data!; - readNotifications = all.data!.filter((n) => n.read); - loading = false; + $: hasMoreUnread = unreadNotifications.length < unreadTotal; + $: hasMoreRead = readNotifications.length < readTotal; + + function totalCountOf(response: Response, fallback: number): number { + const total = Number(response.headers.get('X-Total-Count')); + return Number.isNaN(total) ? fallback : total; + } + + function timeOf(notification: Notification): number { + return new Date(notification.timestamp ?? 0).getTime(); + } + + /** Insert into a newest-first list, keeping it ordered. */ + function insertNewestFirst( + notifications: Notification[], + notification: Notification, + hasMore: boolean + ): Notification[] { + const index = notifications.findIndex((n) => timeOf(n) < timeOf(notification)); + if (index === -1) { + // Older than everything loaded: if further pages exist it belongs to one + // of them, so leave it to be fetched in order rather than tacking it on. + return hasMore ? notifications : [...notifications, notification]; + } + return [...notifications.slice(0, index), notification, ...notifications.slice(index)]; + } + + async function loadUnread({ reset = false } = {}) { + const offset = reset ? 0 : unreadNotifications.length; + const { data, response } = await client.GET('/api/v1/notification/unread', { + params: { query: { limit: PAGE_SIZE, offset } } + }); + if (!data) return; + + unreadNotifications = reset ? data : [...unreadNotifications, ...data]; + unreadTotal = totalCountOf(response, unreadNotifications.length); + } + + async function loadRead({ reset = false } = {}) { + const offset = reset ? 0 : readNotifications.length; + const { data, response } = await client.GET('/api/v1/notification', { + params: { query: { limit: PAGE_SIZE, offset, read: true } } + }); + if (!data) return; + + readNotifications = reset ? data : [...readNotifications, ...data]; + readTotal = totalCountOf(response, readNotifications.length); + } + + async function loadMoreUnread() { + loadingMoreUnread = true; + try { + await loadUnread(); + } finally { + loadingMoreUnread = false; + } + } + + async function loadMoreRead() { + loadingMoreRead = true; + try { + await loadRead(); + } finally { + loadingMoreRead = false; + } + } + + async function toggleShowRead() { + showRead = !showRead; + // Read notifications are the bulk of the table, so they are not fetched at + // all until the section is opened. + if (showRead && readNotifications.length === 0) { + loadingMoreRead = true; + try { + await loadRead({ reset: true }); + } finally { + loadingMoreRead = false; + } + } } async function markAsRead(notificationId: string) { const { response } = await client.PATCH('/api/v1/notification/{notification_id}/read', { params: { path: { notification_id: notificationId } } }); + if (!response.ok) return; - if (response.ok) { - const notification = unreadNotifications.find((n) => n.id === notificationId); - if (notification) { - notification.read = true; - readNotifications = [notification, ...readNotifications]; - unreadNotifications = unreadNotifications.filter((n) => n.id !== notificationId); - } + const notification = unreadNotifications.find((n) => n.id === notificationId); + if (!notification) return; + + notification.read = true; + unreadNotifications = unreadNotifications.filter((n) => n.id !== notificationId); + unreadTotal = Math.max(0, unreadTotal - 1); + readTotal += 1; + if (showRead) { + readNotifications = insertNewestFirst(readNotifications, notification, hasMoreRead); } } @@ -43,36 +127,33 @@ const { response } = await client.PATCH('/api/v1/notification/{notification_id}/unread', { params: { path: { notification_id: notificationId } } }); + if (!response.ok) return; - if (response.ok) { - const notification = readNotifications.find((n) => n.id === notificationId); - if (notification) { - notification.read = false; - unreadNotifications = [notification, ...unreadNotifications]; - readNotifications = readNotifications.filter((n) => n.id !== notificationId); - } - } + const notification = readNotifications.find((n) => n.id === notificationId); + if (!notification) return; + + notification.read = false; + readNotifications = readNotifications.filter((n) => n.id !== notificationId); + readTotal = Math.max(0, readTotal - 1); + unreadTotal += 1; + unreadNotifications = insertNewestFirst(unreadNotifications, notification, hasMoreUnread); } async function markAllAsRead() { - if (unreadNotifications.length === 0) return; + if (unreadTotal === 0) return; try { markingAllAsRead = true; - const promises = unreadNotifications.map((notification) => - client.PATCH('/api/v1/notification/{notification_id}/read', { - params: { path: { notification_id: notification.id! } } - }) - ); - - await Promise.all(promises); - - // Move all unread to read - readNotifications = [ - ...unreadNotifications.map((n) => ({ ...n, read: true })), - ...readNotifications - ]; + // One request: the client no longer holds every notification, so it cannot + // mark them all read by looping over the ones it happens to have. + const { response } = await client.PATCH('/api/v1/notification/read'); + if (!response.ok) return; + unreadNotifications = []; + unreadTotal = 0; + readNotifications = []; + readTotal = 0; + if (showRead) await loadRead({ reset: true }); } catch (error) { console.error('Failed to mark all notifications as read:', error); } finally { @@ -81,9 +162,16 @@ } onMount(() => { - fetchNotifications(); + loading = true; + loadUnread({ reset: true }).finally(() => (loading = false)); - const interval = setInterval(fetchNotifications, 30000); + const interval = setInterval(() => { + if (loading || markingAllAsRead) return; + // Only refresh while a single page is shown, so polling cannot discard + // pages the user has loaded. + if (unreadNotifications.length > PAGE_SIZE) return; + loadUnread({ reset: true }); + }, 30000); return () => clearInterval(interval); }); @@ -117,7 +205,7 @@

Notifications

- {#if unreadNotifications.length > 0} + {#if unreadTotal > 0}
{/each} + + {#if hasMoreUnread} +
+ +

+ Showing {unreadNotifications.length} of {unreadTotal} +

+
+ {/if} {/if}
@@ -214,11 +321,17 @@ {#if showRead}
{#if readNotifications.length === 0} -
-

No read notifications

-
+ {#if loadingMoreRead} +
+
+
+ {:else} +
+

No read notifications

+
+ {/if} {:else}
{#each readNotifications as notification (notification.id)} @@ -257,6 +370,25 @@
{/each}
+ + {#if hasMoreRead} +
+ +

+ Showing {readNotifications.length} of {readTotal} +

+
+ {/if} {/if} {/if}