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 <noreply@anthropic.com>
This commit is contained in:
Kyle 2026-02-25 07:48:45 +08:00
parent 15c99152d3
commit 4a98b67b0b
5 changed files with 24 additions and 3 deletions

View File

@ -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,
)

View File

@ -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())

View File

@ -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)

View File

@ -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

View File

@ -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() {
<CardContent>
<div>
<Label>Accent Color</Label>
<div className="grid grid-cols-5 gap-3 mt-3">
<div className="grid grid-cols-4 gap-3 mt-3">
{accentColors.map((color) => (
<button
key={color.name}