Skip to content

fix(leaderboard): clamp zap limit values#333

Merged
ralyodio merged 4 commits into
profullstack:masterfrom
Jorel97:codex/fix-zap-leaderboard-limit-332
May 29, 2026
Merged

fix(leaderboard): clamp zap limit values#333
ralyodio merged 4 commits into
profullstack:masterfrom
Jorel97:codex/fix-zap-leaderboard-limit-332

Conversation

@Jorel97
Copy link
Copy Markdown
Contributor

@Jorel97 Jorel97 commented May 29, 2026

Summary

  • clamp /api/leaderboard/zaps limit query values to the supported 1..50 range
  • fall back to the default 25 when limit is non-numeric
  • add route coverage for negative and non-numeric limits

Fixes #332.

Verification

  • Added focused Vitest coverage for limit=-5 and limit=abc.
  • Not run locally: this Codex workspace has Node but no npm/package runner available, so I could not execute the Vitest file here.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 29, 2026

Greptile Summary

This PR fixes an input-validation gap in the zap leaderboard endpoint by clamping the limit query parameter to the 1–50 supported range and falling back to 25 for non-numeric values. It also adds Vitest coverage for the two newly guarded edge cases.

  • route.ts: parseInt now uses radix 10, a Number.isFinite guard replaces the silent NaN path (which previously produced an empty slice), and Math.max(parsedLimit, 1) adds the missing lower-bound clamp.
  • route.test.ts: Two new tests verify that limit=-5 returns exactly 1 row and limit=abc falls back to the default 25-row window; the Supabase mock correctly exposes a chainable gte method so future period-filter tests won't fail.

Confidence Score: 5/5

Safe to merge — the change is a narrow, well-contained input-validation fix with no impact on any other code path.

The limit-clamping logic is correct: NaN from non-numeric input is caught by Number.isFinite, the lower bound is properly enforced by Math.max(parsedLimit, 1), and the existing Math.min(..., 50) upper bound is preserved. The tests directly exercise both new code paths, and the mock's chainable gte design means period-filtered variants of these tests would also work. No regressions are introduced in the surrounding aggregation or profile-fetching logic.

No files require special attention.

Important Files Changed

Filename Overview
src/app/api/leaderboard/zaps/route.ts Adds radix-10 parseInt, Number.isFinite NaN guard, and Math.max lower-bound clamp; correctly fixes negative and non-numeric limit handling.
src/app/api/leaderboard/zaps/route.test.ts New Vitest file covering negative and non-numeric limit inputs; mock correctly returns a chainable query object (gte + then) that handles both the no-period and period-filtered code paths.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GET /api/leaderboard/zaps] --> B[Parse limit param]
    B --> C{parseInt result\nfinite?}
    C -- Yes --> D["Math.min(Math.max(n, 1), 50)"]
    C -- No / NaN --> E[limit = 25 default]
    D --> F[Fetch zaps from Supabase]
    E --> F
    F --> G{dateFilter set?}
    G -- period=week/month --> H[query.gte created_at]
    G -- period=all --> I[query unchanged]
    H --> J[Await query]
    I --> J
    J --> K[Aggregate per-user stats]
    K --> L[Sort by total_sats, slice 0..limit]
    L --> M{Results empty?}
    M -- Yes --> N[Return leaderboard: empty]
    M -- No --> O[Fetch profiles by user IDs]
    O --> P[Build ranked leaderboard]
    P --> Q[Return JSON response]
Loading

Reviews (2): Last reviewed commit: "test(leaderboard): make zap mock chainab..." | Re-trigger Greptile

Comment thread src/app/api/leaderboard/zaps/route.ts Outdated
Comment thread src/app/api/leaderboard/zaps/route.test.ts Outdated
@Jorel97
Copy link
Copy Markdown
Contributor Author

Jorel97 commented May 29, 2026

Addressed the Greptile notes in d4d37be: cleaned the garbled JSDoc dash and made the zaps test mock chainable so future period-filter tests can call .gte() without refactoring the mock.

@ralyodio ralyodio merged commit ad794f6 into profullstack:master May 29, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Zap leaderboard accepts invalid limit values

2 participants