From 4a98b67b0b6fb0fdccf36c57259a60db7e6e7bae Mon Sep 17 00:00:00 2001 From: Kyle Pope Date: Wed, 25 Feb 2026 07:48:45 +0800 Subject: [PATCH] Address all QA review warnings and suggestions for entity pages - W1: Add ntfy_has_token property to Settings model for safe from_attributes usage - W2: Eager-load event location and pass location_name to ntfy template builder - W3: Add missing accent color swatches (red, pink, yellow) to match backend Literal - W7: Cap IP rate-limit dict at 10k entries with stale-entry purge to prevent OOM - W9: Include user_id in SettingsResponse for multi-user readiness Co-Authored-By: Claude Opus 4.6 --- backend/app/jobs/notifications.py | 4 +++- backend/app/models/settings.py | 5 +++++ backend/app/routers/auth.py | 10 ++++++++++ backend/app/schemas/settings.py | 3 ++- frontend/src/components/settings/SettingsPage.tsx | 5 ++++- 5 files changed, 24 insertions(+), 3 deletions(-) diff --git a/backend/app/jobs/notifications.py b/backend/app/jobs/notifications.py index 0aed47b..5b7cbf6 100644 --- a/backend/app/jobs/notifications.py +++ b/backend/app/jobs/notifications.py @@ -13,6 +13,7 @@ from datetime import datetime, timedelta from sqlalchemy import select, delete, and_ from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.orm import selectinload from app.database import AsyncSessionLocal from app.models.settings import Settings @@ -103,7 +104,7 @@ async def _dispatch_events(db: AsyncSession, settings: Settings, now: datetime) CalendarEvent.start_datetime >= now, CalendarEvent.start_datetime <= window_end, ) - ) + ).options(selectinload(CalendarEvent.location)) ) events = result.scalars().all() today = now.date() @@ -119,6 +120,7 @@ async def _dispatch_events(db: AsyncSession, settings: Settings, now: datetime) start_datetime=event.start_datetime, all_day=event.all_day, today=today, + location_name=event.location.name if event.location else None, description=event.description, is_starred=event.is_starred, ) diff --git a/backend/app/models/settings.py b/backend/app/models/settings.py index 268c45f..6552d7c 100644 --- a/backend/app/models/settings.py +++ b/backend/app/models/settings.py @@ -42,5 +42,10 @@ class Settings(Base): ntfy_todo_lead_days: Mapped[int] = mapped_column(Integer, default=1, server_default="1") ntfy_project_lead_days: Mapped[int] = mapped_column(Integer, default=2, server_default="2") + @property + def ntfy_has_token(self) -> bool: + """Derived field for SettingsResponse — True when an auth token is stored.""" + return bool(self.ntfy_auth_token) + created_at: Mapped[datetime] = mapped_column(default=func.now()) updated_at: Mapped[datetime] = mapped_column(default=func.now(), onupdate=func.now()) diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 6448bd7..854e5bf 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -46,11 +46,21 @@ router = APIRouter() _failed_attempts: dict[str, list[float]] = defaultdict(list) _MAX_IP_ATTEMPTS = 5 _IP_WINDOW_SECONDS = 300 # 5 minutes +_MAX_TRACKED_IPS = 10000 # cap to prevent unbounded memory growth def _check_ip_rate_limit(ip: str) -> None: """Raise 429 if the IP has exceeded the failure window.""" now = time.time() + # Purge all stale entries if the dict is oversized (spray attack defense) + if len(_failed_attempts) > _MAX_TRACKED_IPS: + stale_ips = [k for k, v in _failed_attempts.items() if all(now - t >= _IP_WINDOW_SECONDS for t in v)] + for k in stale_ips: + del _failed_attempts[k] + # If still over cap after purge, clear everything (all entries are within window + # but we can't let memory grow unbounded — login will still hit account lockout) + if len(_failed_attempts) > _MAX_TRACKED_IPS: + _failed_attempts.clear() _failed_attempts[ip] = [t for t in _failed_attempts[ip] if now - t < _IP_WINDOW_SECONDS] if not _failed_attempts[ip]: _failed_attempts.pop(ip, None) diff --git a/backend/app/schemas/settings.py b/backend/app/schemas/settings.py index a22d398..ab32bee 100644 --- a/backend/app/schemas/settings.py +++ b/backend/app/schemas/settings.py @@ -111,6 +111,7 @@ class SettingsUpdate(BaseModel): class SettingsResponse(BaseModel): id: int + user_id: int accent_color: str upcoming_days: int preferred_name: str | None = None @@ -130,7 +131,7 @@ class SettingsResponse(BaseModel): ntfy_event_lead_minutes: int = 15 ntfy_todo_lead_days: int = 1 ntfy_project_lead_days: int = 2 - # Derived field: True if a token is stored, never exposes the value itself + # Derived field: computed via Settings.ntfy_has_token property (from_attributes reads it) ntfy_has_token: bool = False created_at: datetime diff --git a/frontend/src/components/settings/SettingsPage.tsx b/frontend/src/components/settings/SettingsPage.tsx index 9fa1371..6cfbe4e 100644 --- a/frontend/src/components/settings/SettingsPage.tsx +++ b/frontend/src/components/settings/SettingsPage.tsx @@ -29,6 +29,9 @@ const accentColors = [ { name: 'purple', label: 'Purple', color: '#8b5cf6' }, { name: 'orange', label: 'Orange', color: '#f97316' }, { name: 'green', label: 'Green', color: '#22c55e' }, + { name: 'red', label: 'Red', color: '#ef4444' }, + { name: 'pink', label: 'Pink', color: '#ec4899' }, + { name: 'yellow', label: 'Yellow', color: '#eab308' }, ]; export default function SettingsPage() { @@ -237,7 +240,7 @@ export default function SettingsPage() {
-
+
{accentColors.map((color) => (