From a2c1058f9ca17d9c488dc3f3b09f62b3b0eed9d2 Mon Sep 17 00:00:00 2001 From: Kyle Pope Date: Sun, 15 Mar 2026 01:46:11 +0800 Subject: [PATCH] Fix QA findings: single UNION query, weekly validation, nginx docs W-01: Consolidate get_accessible_calendar_ids to single UNION query instead of two separate DB round-trips. W-02: Document that nginx rate limit on /api/events applies to all methods (30r/m generous enough for GET polling at 2r/m). W-03: Add weekly rule validation for consistency with other rule types. Co-Authored-By: Claude Opus 4.6 --- backend/app/schemas/calendar_event.py | 2 ++ backend/app/services/calendar_sharing.py | 14 ++++++++------ frontend/nginx.conf | 4 +++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/backend/app/schemas/calendar_event.py b/backend/app/schemas/calendar_event.py index 0486ec5..be14716 100644 --- a/backend/app/schemas/calendar_event.py +++ b/backend/app/schemas/calendar_event.py @@ -22,6 +22,8 @@ class RecurrenceRule(BaseModel): """Enforce required fields per rule type.""" if self.type == "every_n_days" and self.interval is None: raise ValueError("every_n_days rule requires 'interval'") + if self.type == "weekly" and self.weekday is None: + raise ValueError("weekly rule requires 'weekday'") if self.type == "monthly_nth_weekday": if self.week is None or self.weekday is None: raise ValueError("monthly_nth_weekday rule requires both 'week' and 'weekday'") diff --git a/backend/app/services/calendar_sharing.py b/backend/app/services/calendar_sharing.py index 1f94ed7..da1d71a 100644 --- a/backend/app/services/calendar_sharing.py +++ b/backend/app/services/calendar_sharing.py @@ -22,14 +22,16 @@ LOCK_DURATION_MINUTES = 5 async def get_accessible_calendar_ids(user_id: int, db: AsyncSession) -> list[int]: """Return all calendar IDs the user can access (owned + accepted shared memberships).""" - owned = await db.execute(select(Calendar.id).where(Calendar.user_id == user_id)) - shared = await db.execute( - select(CalendarMember.calendar_id).where( - CalendarMember.user_id == user_id, - CalendarMember.status == "accepted", + result = await db.execute( + select(Calendar.id).where(Calendar.user_id == user_id) + .union( + select(CalendarMember.calendar_id).where( + CalendarMember.user_id == user_id, + CalendarMember.status == "accepted", + ) ) ) - return [r[0] for r in owned.all()] + [r[0] for r in shared.all()] + return [r[0] for r in result.all()] async def get_user_permission(db: AsyncSession, calendar_id: int, user_id: int) -> str | None: diff --git a/frontend/nginx.conf b/frontend/nginx.conf index 73087cc..85d709c 100644 --- a/frontend/nginx.conf +++ b/frontend/nginx.conf @@ -124,7 +124,9 @@ server { include /etc/nginx/proxy-params.conf; } - # Event creation — rate-limited to prevent DB flooding via recurrence amplification + # Event creation — rate-limited to prevent DB flooding via recurrence amplification. + # Note: exact match applies to GET+POST; 30r/m with burst=10 is generous enough + # for polling (2r/m) and won't affect reads even with multiple tabs. location = /api/events { limit_req zone=event_create_limit burst=10 nodelay; limit_req_status 429;