Skip to content

fix(tags): clamp popular limit values#361

Open
Zekbot001 wants to merge 1 commit into
profullstack:masterfrom
Zekbot001:money/ugig-popular-tags-limit
Open

fix(tags): clamp popular limit values#361
Zekbot001 wants to merge 1 commit into
profullstack:masterfrom
Zekbot001:money/ugig-popular-tags-limit

Conversation

@Zekbot001
Copy link
Copy Markdown

Summary

  • replace the popular-tags route's ad hoc limit expression with the existing bounded pagination parser
  • clamp limit=0 to 1 instead of accidentally expanding it to the default 50
  • add focused route coverage for zero, malformed, and oversized limit values

Fixes #360.

Paid task context: https://ugig.net/gigs/abd6b2a0-e728-48cf-a46f-f99e419ed94e

Verification

  • .\node_modules\.bin\vitest.CMD run src/app/api/tags/popular/route.test.ts
  • .\node_modules\.bin\eslint.CMD src/app/api/tags/popular/route.ts src/app/api/tags/popular/route.test.ts
  • git diff --cached --check

Existing upstream check blockers

  • .\node_modules\.bin\tsc.CMD --noEmit reaches unrelated existing errors in src/app/api/affiliates/offers/route.test.ts:53 and src/app/api/applications/[id]/route.test.ts:21.
  • corepack pnpm run build compiles successfully and progresses through static generation when given a process-local dummy OpenAI key, then stops because the local checkout has no Supabase service-role configuration.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 31, 2026

Greptile Summary

This PR fixes a subtle falsy-zero bug in the popular-tags route where limit=0 was silently expanded to the default 50, by replacing the inline parseInt(…) || 50 expression with the shared parsePaginationParam utility that correctly clamps zero to the specified minimum. A new test file provides targeted coverage for the three edge-case inputs (zero, malformed, and oversized limits).

  • route.ts: The old expression parseInt(searchParams.get("limit") || "50", 10) || 50 treated "0" as falsy and fell back to 50; the new parsePaginationParam("0", 50, 1, 200) correctly returns 1.
  • route.test.ts: Three new tests verify limit=0→1, limit=abc→50 (default), and limit=999→200 against the mocked Supabase client.

Confidence Score: 5/5

Safe to merge — the change is a one-line swap to a well-tested utility with correct clamping semantics, and the new tests confirm all three boundary cases.

The fix is minimal and focused: the parsePaginationParam utility already handles null, NaN, zero, and oversized values correctly, and the three new tests exercise exactly the edge cases the bug report described. No other logic in the route was changed, and the existing Supabase query path is untouched.

No files require special attention.

Important Files Changed

Filename Overview
src/app/api/tags/popular/route.ts Replaces the ad-hoc limit expression with parsePaginationParam; correctly clamps limit=0 to 1 and removes the `
src/app/api/tags/popular/route.test.ts New test file adding focused coverage for zero, malformed, and oversized limit values; mock setup and assertions are consistent with the route's behavior.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Route as GET /api/tags/popular
    participant parsePaginationParam
    participant Supabase

    Client->>Route: "GET ?limit=<value>"
    Route->>parsePaginationParam: parsePaginationParam(value, 50, 1, 200)
    Note over parsePaginationParam: null/blank → 50 (default)<br/>NaN → 50 (default)<br/>0 → 1 (clamped to min)<br/>999 → 200 (clamped to max)
    parsePaginationParam-->>Route: bounded limit
    Route->>Supabase: from("gigs").select("skills_required").eq("status","active")
    Supabase-->>Route: gigs[]
    Route->>Supabase: from("tag_follows").select("tag")
    Supabase-->>Route: follows[]
    Note over Route: count tags, merge, sort by<br/>follower_count desc, gig_count desc
    Route->>Route: tags.slice(0, limit)
    Route-->>Client: "{ tags: limited[] }"
Loading

Reviews (1): Last reviewed commit: "fix(tags): clamp popular limit values" | Re-trigger Greptile

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.

Clamp zero popular-tags limits instead of expanding to the default

1 participant