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/tests/conftest.py b/tests/conftest.py index a505b63c..49cf034b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -22,6 +22,7 @@ from media_manager.movies import models as movie_models from media_manager.movies.schemas import Movie as MovieSchema from media_manager.movies.schemas import MovieId +from media_manager.notification import models as notification_models from media_manager.tv import models as tv_models from media_manager.tv.schemas import Episode as EpisodeSchema from media_manager.tv.schemas import EpisodeId, SeasonId, ShowId @@ -31,7 +32,7 @@ # Importing the model modules is what registers their tables on ``Base.metadata``; # binding them here keeps the imports from looking unused. Note that the indexer # models are deliberately not imported: they use a PostgreSQL-only ARRAY column. -_MODEL_MODULES = (movie_models, tv_models) +_MODEL_MODULES = (movie_models, notification_models, tv_models) @pytest.fixture 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/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}