-
Notifications
You must be signed in to change notification settings - Fork 16
ENS Referrals API Versioning #1554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: fb3a04d The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughIntroduces API versioning for ENSAnalytics referral endpoints by creating new v1 routes at Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.test.ts`:
- Around line 140-175: Add a new test verifying the /referrers error path by
mocking middleware.referrerLeaderboardMiddleware to set
c.set("referrerLeaderboard", new Error("...")) and then calling
client.referrers.$get(...) and deserializing with
deserializeReferrerLeaderboardPageResponse; assert the response.responseCode
equals ReferrerLeaderboardPageResponseCodes.Error and that response.error and
response.errorMessage match the expected "Internal Server Error" and "Failed to
load referrer leaderboard data." values respectively so the handler's lines
handling Error referrerLeaderboard are covered.
- Around line 158-172: The test's expectedResponse.pageContext is missing
startIndex and endIndex; update the expected object in
apps/ensapi/src/handlers/ensanalytics-api-v1.test.ts so the pageContext includes
startIndex: 0 and endIndex: 0 (alongside hasNext, hasPrev, recordsPerPage, page,
totalPages, totalRecords) so it satisfies ReferrerLeaderboardPageResponseOk and
validates that the handler returns those fields even for
emptyReferralLeaderboard when using ReferrerLeaderboardPageResponseCodes.Ok.
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.ts`:
- Around line 69-71: The current invariant check throws an Error when
c.var.referrerLeaderboard is undefined (referrerLeaderboardMiddleware missing),
which bypasses the endpoint's serialized error response; instead, replace the
throw with the same serialized 500 response used elsewhere (call the existing
errorResponse helper or the handler's JSON error payload) so the endpoint
adheres to the OpenAPI contract; apply the same change to the other invariant at
lines referencing c.var.referrerLeaderboard (the similar check around 142-144)
so both checks return a structured 500 response rather than throwing.
- Around line 74-83: The /referrers handler returns HTTP 500 when
c.var.referrerLeaderboard is an Error but the /referrers/:referrer handler
returns 503 for the same condition; update the first handler to return 503 for
consistency and to reflect a temporary service-unavailable condition by changing
the status argument on the c.json call that sends
serializeReferrerLeaderboardPageResponse({ responseCode:
ReferrerLeaderboardPageResponseCodes.Error, ... }) from 500 to 503 so both
endpoints behave the same.
- Around line 74-83: The referrer leaderboard error message in the
c.var.referrerLeaderboard handling (inside the
serializeReferrerLeaderboardPageResponse call using
ReferrerLeaderboardPageResponseCodes.Error) is inconsistent with the referrer
detail endpoint; change the error string "Failed to load referrer leaderboard
data." to the same canonical message used by the referrer detail endpoint
("Referrer leaderboard data has not been successfully cached yet.") or, better,
extract a shared constant (e.g., REFERRER_LEADERBOARD_NOT_CACHED_MESSAGE) and
use it in both serializeReferrerLeaderboardPageResponse and the referrer detail
response path so both endpoints return identical wording.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces explicit API versioning for ENS Referrals endpoints by duplicating the existing handler and tests under a new /ensanalytics/v1 route. The existing /ensanalytics routes are preserved as implicit v0, ensuring zero breaking changes while preparing infrastructure for future v1 evolution.
Changes:
- Added new v1 handler (
ensanalytics-api-v1.ts) that duplicates existing v0 implementation - Added corresponding v1 test suite (
ensanalytics-api-v1.test.ts) - Mounted v1 routes at
/ensanalytics/v1in the main application index - Minor cleanup: removed incorrect comment from v0 test file
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
apps/ensapi/src/index.ts |
Added v1 route mounting at /ensanalytics/v1 while preserving existing v0 routes |
apps/ensapi/src/handlers/ensanalytics-api-v1.ts |
Complete duplicate of v0 handler with only string changes in logger name, error messages, and API summaries |
apps/ensapi/src/handlers/ensanalytics-api-v1.test.ts |
Complete duplicate of v0 tests adapted for v1 endpoints |
apps/ensapi/src/handlers/ensanalytics-api.test.ts |
Removed incorrect comment header |
.changeset/tough-phones-cry.md |
Documented the API versioning change as a minor version bump |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryIntroduces API versioning for ENSAnalytics referral endpoints by creating duplicate v1 routes while preserving existing v0 routes. Key Changes:
Architecture: Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant ENSApi as ENSApi Server
participant Router as Router (index.ts)
participant V0Handler as ensanalytics-api (v0)
participant V1Handler as ensanalytics-api-v1 (v1)
participant Middleware as referrerLeaderboardMiddleware
participant Cache as referrerLeaderboardCache
Note over Client,Cache: API Versioning - Both v0 and v1 Routes Available
Client->>ENSApi: GET /ensanalytics/referrers
ENSApi->>Router: Route request
Router->>V0Handler: Forward to v0 handler
V0Handler->>Middleware: Apply middleware
Middleware->>Cache: Get leaderboard data
Cache-->>Middleware: Return cached data
Middleware-->>V0Handler: Set context variable
V0Handler->>V0Handler: Process request & paginate
V0Handler-->>Client: Return leaderboard page
Client->>ENSApi: GET /ensanalytics/v1/referrers
ENSApi->>Router: Route request
Router->>V1Handler: Forward to v1 handler
V1Handler->>Middleware: Apply middleware
Middleware->>Cache: Get leaderboard data
Cache-->>Middleware: Return cached data
Middleware-->>V1Handler: Set context variable
V1Handler->>V1Handler: Process request & paginate
V1Handler-->>Client: Return leaderboard page
Note over V0Handler,V1Handler: Currently identical implementations<br/>Future PRs will evolve v1 with breaking changes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Caution Docstrings generation - FAILED No docstrings were generated. |
lightwalker-eth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Goader Looks good! Thank you! 👍
ENSAnalytics API Versioning: Add Explicit v1 Routes
closes: #1520
Reviewer Focus (Read This First)
What reviewers should focus on
ensanalytics-api-v1.tshandler - exact duplicate (apart from a few strings in errors and similar) of 'ensanalytics-api.ts`index.ts- both v0 and v1 now availableDiff between `ensanalytics-api.ts` and `ensanalytics-api-v1.ts`
Diff between `ensanalytics-api.test.ts` and `ensanalytics-api-v1.test.ts`
Problem & Motivation
Why this exists
What Changed (Concrete)
What actually changed
apps/ensapi/src/handlers/ensanalytics-api-v1.ts- duplicate of existing handlerapps/ensapi/src/handlers/ensanalytics-api-v1.test.ts- duplicate of existing testsapps/ensapi/src/index.ts- added v1 route mounting at/ensanalytics/v1Design & Planning
How this approach was chosen
Both v0 and v1 are identical initially. Future PRs will evolve v1 with breaking changes while v0 remains frozen to support current ENSAwards.
Self-Review
What you caught yourself
Cross-Codebase Alignment
Related code you checked
ensanalytics,referrer,referralDownstream & Consumer Impact
Who this affects and how
/v1/prefix for explicit versioningTesting Evidence
How this was validated
Scope Reductions
What you intentionally didn't do
No changes to v1 implementation.
Risk Analysis
How this could go wrong
Very low risk - purely additive change with no modifications to existing behavior.
Pre-Review Checklist (Blocking)