feat: add limit/pagination to events endpoint (#147) #156
No reviewers
Labels
No Label
ai-changes-requested
ai-failed
ai-merged
ai-pr-opened
ai-reviewed
ai-reviewing
ai-reviewing
ai-working
bug
enhancement
evolution
performance
phase-0
phase-1a
phase-1b
phase-1c
phase-1d
security
tech-debt
todo
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: cal/paper-dynasty-database#156
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "issue/147-feat-add-limit-pagination-to-events-endpoint"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #147
Adds an optional
limitquery parameter (default 100, max 500) toGET /api/v2/events.Changes
app/routers_v2/events.py: Addedlimit: Optional[int] = 100parameter tov1_events_get. Appliedall_events.limit(max(0, min(limit, 500)))after all filters, before the CSV/JSON branch — consistent with the pattern used in notifications, scout_claims, rewards, scout_opportunities, and batstats endpoints.Test results
No test suite. Change verified by reading back the modified file and confirming the diff is minimal and correct. Ruff pre-commit hook passed.
AI Code Review
Files Reviewed
app/routers_v2/events.py(modified)Findings
Correctness
limit: Optional[int] = 100added tov1_events_getsignature.Optionalalready imported at line 2 — no missing import.all_events.limit(max(0, min(limit, 500)))applied after all filters (name,in_desc,active), before theif csv:branch — both CSV and JSON response paths correctly honour the limit.max(0, min(limit, 500))guards negative values (floors to 0) and oversized values (caps at 500). Identical to the pattern in notifications, scout_claims, rewards, scout_opportunities, and batstats endpoints.Security
limitis typedOptional[int]; FastAPI validates the type before the function executes. ORM handles query parameterisation.Style & Conventions
Suggestions
all_events.count()(line 55) is called after.limit()is applied, so thecountfield in the JSON response reflects the limited count, not the total number of matching events. This is the same pre-existing pattern across all other paginated endpoints — a follow-up issue to add atotalfield project-wide would address it consistently.limit=0returns an empty response even when events exist (lower bound of the clamp). Same pedantic edge case present in all prior pagination PRs — non-blocking.Verdict: APPROVED
Clean, minimal, correct. Follows the pagination pattern established across the five prior endpoints without deviation. No blocking issues.
Automated review by Claude PR Reviewer
Approved — reviewer verdict is APPROVED, implementation is clean and consistent with the established pagination pattern.
Re-approving after rebase
Re-approving on rebased head — same clean, minimal change. Reviewer verdict remains APPROVED.
Re-approving after another rebase
Approved on rebased head.
rebase
Approved.
rebase
Approved.