feat: add live Twitter/X stats under submission details panel in spon…#1427
feat: add live Twitter/X stats under submission details panel in spon…#1427mansiverma897993 wants to merge 2 commits into
Conversation
|
@mansiverma897993 is attempting to deploy a commit to the Superteam Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a Twitter/X tweet stats API route, a ChangesTweet Engagement Stats
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant DetailsView
participant TweetStats
participant TweetStatsAPI
participant TwitterAPI
DetailsView->>TweetStats: render with tweet URL
TweetStats->>TweetStatsAPI: GET /api/twitter/tweet-stats?url=...
TweetStatsAPI->>TweetStatsAPI: validate and extract tweet ID
alt token missing or fetch fails
TweetStatsAPI-->>TweetStats: zeroed metrics, unavailable
else success
TweetStatsAPI->>TwitterAPI: fetch public_metrics
TwitterAPI-->>TweetStatsAPI: public_metrics
TweetStatsAPI-->>TweetStats: mapped engagement metrics
end
TweetStats-->>DetailsView: render stats
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
src/features/sponsor-dashboard/components/Submissions/TweetStats.tsx (2)
31-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid
anyfor the caught error.As per coding guidelines: "Outside of generic functions, use
anytype extremely sparingly."♻️ Proposed fix
- } catch (err: any) { + } catch (err: unknown) { if (active) { - setError(err.message || 'Error loading stats'); + setError(err instanceof Error ? err.message : 'Error loading stats'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/sponsor-dashboard/components/Submissions/TweetStats.tsx` at line 31, The catch block in TweetStats should not use any for the thrown error. Update the error handling in the component’s try/catch to use a safer caught-error type (or narrow from unknown) and adjust any references inside the catch body accordingly, keeping the change localized to the TweetStats component.Source: Coding guidelines
4-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMark
TweetMetricsfields asreadonly.As per coding guidelines: "Use
readonlyproperties for object types by default in TypeScript to prevent accidental mutation at runtime."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/sponsor-dashboard/components/Submissions/TweetStats.tsx` around lines 4 - 10, The TweetMetrics interface currently allows mutation of its fields, which should be avoided by default. Update the TweetMetrics type in TweetStats to mark each property as readonly, preserving the existing field names (views, likes, retweets, comments, isMocked) while preventing accidental reassignment.Source: Coding guidelines
src/pages/api/twitter/tweet-stats.ts (5)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse top-level
import typeinstead of inlinetypein a named import.As per path instructions: "Prefer top-level
import typeover inlineimport { type ... }syntax."♻️ Proposed fix
-import { type NextApiRequestWithUser } from '`@/features/auth/types`'; +import type { NextApiRequestWithUser } from '`@/features/auth/types`';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/api/twitter/tweet-stats.ts` at line 5, The import in tweet-stats.ts uses inline type syntax inside a named import, which should be converted to top-level type-only import style. Update the import that brings in NextApiRequestWithUser so it uses import type at the top level instead of import { type ... }, matching the project’s import conventions and keeping type-only dependencies explicit.Source: Path instructions
46-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate mock-metric generation logic.
The seed/views/likes/retweets/comments computation is duplicated verbatim in the no-token branch and the API-error fallback branch. Extract into a shared helper.
♻️ Proposed fix
+function generateMockMetrics(tweetId: string) { + const seed = parseInt(tweetId.slice(-4), 10) || 1234; + const views = Math.floor(seed * 1.5) + 120; + const likes = Math.floor(views * 0.08) + 5; + const retweets = Math.floor(likes * 0.15) + 1; + const comments = Math.floor(likes * 0.1) + 1; + return { views, likes, retweets, comments, isMocked: true }; +}Then call
res.status(200).json(generateMockMetrics(tweetId));in both branches.Also applies to: 74-89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/api/twitter/tweet-stats.ts` around lines 46 - 62, The mock metric calculation is duplicated in the bearer-token fallback and the API-error fallback inside tweet-stats.ts. Extract the seed/views/likes/retweets/comments logic into a shared helper (for example, a generateMockMetrics function) and have both the no-token path and the error-handling path call it, then return its result via res.status(200).json(...) to keep the behavior identical in both branches.
32-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeclare explicit return type for the handler function.
handleris a top-level module function; it should declare its return type.As per coding guidelines: "Declare return types for top-level module functions in TypeScript (exception: React components returning JSX)."
♻️ Proposed fix
-async function handler(req: NextApiRequestWithUser, res: NextApiResponse) { +async function handler(req: NextApiRequestWithUser, res: NextApiResponse): Promise<void> {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/api/twitter/tweet-stats.ts` at line 32, The top-level API route function handler is missing an explicit return type, so update its signature to declare the return type directly on handler and keep the type aligned with NextApiResponse usage. Use the handler symbol in tweet-stats.ts as the place to add the return annotation, following the project rule that top-level module functions must declare return types.Source: Coding guidelines
106-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLog error using
safeStringifybefore returning 500.As per path instructions: "Always wrap main logic in try-catch blocks and log errors using safeStringify before returning 500 status."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/api/twitter/tweet-stats.ts` around lines 106 - 107, The error handling in the Twitter/X metrics API handler should log the caught exception with safeStringify before sending the 500 response. Update the try-catch around the main logic in tweet-stats.ts so the logger.error call serializes the error via safeStringify instead of passing the raw error object, then keep returning the existing 500 JSON response. Use the existing logger and safeStringify utilities in the tweet stats request handler.Source: Path instructions
8-30: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHostname check is bypassable via substring matching.
host.includes('twitter.com')also matches spoofed hosts likenottwitter.comortwitter.com.evil.com. No SSRF impact today since the outbound fetch always targetsapi.twitter.comusing only the numerictweetId, but this loosens the intended domain validation and this same logic is duplicated inDetails.tsx'sisTweetStatusUrl.♻️ Proposed fix
- if (!host.includes('twitter.com') && !host.includes('x.com')) { + if ( + host !== 'twitter.com' && !host.endsWith('.twitter.com') && + host !== 'x.com' && !host.endsWith('.x.com') + ) { return null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/api/twitter/tweet-stats.ts` around lines 8 - 30, The hostname validation in extractTweetId is too loose because host.includes(...) can accept spoofed domains; tighten it to only allow the exact Twitter/X hostnames or their proper subdomains, and apply the same fix to the duplicated isTweetStatusUrl logic in Details.tsx. Update the shared parsing/validation behavior so both places reject lookalike hosts such as nottwitter.com or twitter.com.evil.com while still accepting valid twitter.com and x.com URLs.src/features/sponsor-dashboard/components/Submissions/Details.tsx (2)
65-67: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNon-null assertions on
selectedSubmission.Safe today since
isTweetStatusUrlonly returnstruewhenselectedSubmission?.link/.tweetis truthy (implyingselectedSubmissionitself is defined), but the double non-null assertion (selectedSubmission!.link!) is fragile if the check logic changes later.Also applies to: 76-78
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/sponsor-dashboard/components/Submissions/Details.tsx` around lines 65 - 67, The `Details` component is relying on fragile non-null assertions for `selectedSubmission` in the tweet and Bluesky stats branches. Replace `selectedSubmission!.link!`/similar usage with a safe local guard or derived variable after the `isTweetStatusUrl`/bluesky check, and pass the verified link directly to `TweetStats` and the related component so the code stays safe if the predicate logic changes.
19-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate URL-validation logic vs.
extractTweetIdintweet-stats.ts.This function reimplements the same host/status-id parsing already present in
src/pages/api/twitter/tweet-stats.ts(Lines 8-30), including the same.includes()hostname bypass (e.g.nottwitter.compasses). Extract a single shared utility (e.g.parseTweetStatusUrl) usable by both the API route and this component to avoid drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/sponsor-dashboard/components/Submissions/Details.tsx` around lines 19 - 40, The URL parsing logic in isTweetStatusUrl duplicates the tweet-id extraction already used by extractTweetId in tweet-stats.ts, so consolidate both into a shared helper such as parseTweetStatusUrl. Update Details.tsx and the tweet-stats API to call the shared utility instead of maintaining separate hostname/status parsing, and fix the host check so it validates the actual Twitter/X domains rather than relying on a loose .includes() match.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pages/api/twitter/tweet-stats.ts`:
- Around line 35-42: The tweet-stats API handler does not match the endpoint
response conventions: update the logic in the tweet URL validation and
downstream success/error branches in tweet-stats.ts so success returns { data:
result, message: 'Operation successful' } and all error responses include both
error and message fields. Replace the manual tweetUrl checks and tweetId
validation with a Zod schema using safeParse, and return 403 on validation
failure with the schema errors in the error payload. Ensure the same response
shape is used consistently in the main handler paths that build the final tweet
stats response.
- Around line 65-72: The Twitter API call in tweet-stats should not wait
indefinitely; add an AbortController-based timeout around the fetch in the
tweet-stats handler. Update the logic around the external request to
api.twitter.com so the request is aborted after a short, defined duration, and
make sure the handler uses the timeout-aware fetch result/error path in the same
function that builds the tweet stats response.
---
Nitpick comments:
In `@src/features/sponsor-dashboard/components/Submissions/Details.tsx`:
- Around line 65-67: The `Details` component is relying on fragile non-null
assertions for `selectedSubmission` in the tweet and Bluesky stats branches.
Replace `selectedSubmission!.link!`/similar usage with a safe local guard or
derived variable after the `isTweetStatusUrl`/bluesky check, and pass the
verified link directly to `TweetStats` and the related component so the code
stays safe if the predicate logic changes.
- Around line 19-40: The URL parsing logic in isTweetStatusUrl duplicates the
tweet-id extraction already used by extractTweetId in tweet-stats.ts, so
consolidate both into a shared helper such as parseTweetStatusUrl. Update
Details.tsx and the tweet-stats API to call the shared utility instead of
maintaining separate hostname/status parsing, and fix the host check so it
validates the actual Twitter/X domains rather than relying on a loose
.includes() match.
In `@src/features/sponsor-dashboard/components/Submissions/TweetStats.tsx`:
- Line 31: The catch block in TweetStats should not use any for the thrown
error. Update the error handling in the component’s try/catch to use a safer
caught-error type (or narrow from unknown) and adjust any references inside the
catch body accordingly, keeping the change localized to the TweetStats
component.
- Around line 4-10: The TweetMetrics interface currently allows mutation of its
fields, which should be avoided by default. Update the TweetMetrics type in
TweetStats to mark each property as readonly, preserving the existing field
names (views, likes, retweets, comments, isMocked) while preventing accidental
reassignment.
In `@src/pages/api/twitter/tweet-stats.ts`:
- Line 5: The import in tweet-stats.ts uses inline type syntax inside a named
import, which should be converted to top-level type-only import style. Update
the import that brings in NextApiRequestWithUser so it uses import type at the
top level instead of import { type ... }, matching the project’s import
conventions and keeping type-only dependencies explicit.
- Around line 46-62: The mock metric calculation is duplicated in the
bearer-token fallback and the API-error fallback inside tweet-stats.ts. Extract
the seed/views/likes/retweets/comments logic into a shared helper (for example,
a generateMockMetrics function) and have both the no-token path and the
error-handling path call it, then return its result via
res.status(200).json(...) to keep the behavior identical in both branches.
- Line 32: The top-level API route function handler is missing an explicit
return type, so update its signature to declare the return type directly on
handler and keep the type aligned with NextApiResponse usage. Use the handler
symbol in tweet-stats.ts as the place to add the return annotation, following
the project rule that top-level module functions must declare return types.
- Around line 106-107: The error handling in the Twitter/X metrics API handler
should log the caught exception with safeStringify before sending the 500
response. Update the try-catch around the main logic in tweet-stats.ts so the
logger.error call serializes the error via safeStringify instead of passing the
raw error object, then keep returning the existing 500 JSON response. Use the
existing logger and safeStringify utilities in the tweet stats request handler.
- Around line 8-30: The hostname validation in extractTweetId is too loose
because host.includes(...) can accept spoofed domains; tighten it to only allow
the exact Twitter/X hostnames or their proper subdomains, and apply the same fix
to the duplicated isTweetStatusUrl logic in Details.tsx. Update the shared
parsing/validation behavior so both places reject lookalike hosts such as
nottwitter.com or twitter.com.evil.com while still accepting valid twitter.com
and x.com URLs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fa30014e-69d0-45a9-9cd3-5c499e36dfe5
📒 Files selected for processing (3)
src/features/sponsor-dashboard/components/Submissions/Details.tsxsrc/features/sponsor-dashboard/components/Submissions/TweetStats.tsxsrc/pages/api/twitter/tweet-stats.ts
| logger.warn('Twitter/X API bearer token is not configured. Returning simulated mock metrics.'); | ||
| // Generate deterministic mock stats based on the tweetId | ||
| const seed = parseInt(tweetId.slice(-4), 10) || 1234; | ||
| const views = Math.floor(seed * 1.5) + 120; | ||
| const likes = Math.floor(views * 0.08) + 5; | ||
| const retweets = Math.floor(likes * 0.15) + 1; | ||
| const comments = Math.floor(likes * 0.10) + 1; | ||
|
|
||
| return res.status(200).json({ | ||
| views, | ||
| likes, | ||
| retweets, | ||
| comments, | ||
| isMocked: true, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Why are we falling back to showing false stats, if it fails it should either show 0 or no stats available, rather thn showing mock stats
There was a problem hiding this comment.
"Done! I've removed the false/mock stats fallback. Now, if the API call fails or if the bearer token is not configured, it returns 0 metrics, and the UI displays an 'Unavailable' badge instead."
| // Fallback to mock data if API limits or errors occur | ||
| const seed = parseInt(tweetId.slice(-4), 10) || 1234; | ||
| const views = Math.floor(seed * 1.5) + 120; | ||
| const likes = Math.floor(views * 0.08) + 5; | ||
| const retweets = Math.floor(likes * 0.15) + 1; | ||
| const comments = Math.floor(likes * 0.10) + 1; | ||
| return res.status(200).json({ | ||
| views, | ||
| likes, | ||
| retweets, | ||
| comments, | ||
| isMocked: true, | ||
| }); | ||
| } |
|
Hi @RevTpark, I have updated the PR to resolve all the requested changes: Removed False Stats: Replaced the deterministic mock stats fallback entirely. If the API token is not configured or if the request fails, the API now returns 0 metrics with isAvailable: false, and the UI displays an "Unavailable" badge. |
|
hey @mansiverma897993 i think you forgot to push the changes, i can only see 1 commit on this branch. |
|
Hi @RevTpark, Apologies for the oversight! I have successfully pushed the latest changes to the branch now. Here is a summary of the fixes updated in the PR:
Please take another look. Thank you! |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/pages/api/twitter/tweet-stats.ts (2)
47-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeclare an explicit return type for
handler.Top-level module functions should have declared return types. Add
Promise<void>.♻️ Proposed change
-async function handler(req: NextApiRequestWithUser, res: NextApiResponse) { +async function handler( + req: NextApiRequestWithUser, + res: NextApiResponse, +): Promise<void> {As per coding guidelines: "Declare return types for top-level module functions in TypeScript (exception: React components returning JSX)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/api/twitter/tweet-stats.ts` at line 47, The top-level module function handler is missing an explicit return type, so add Promise<void> to its declaration. Update the handler signature in the NextApiRequestWithUser / NextApiResponse API route so it clearly declares its return type while preserving the existing logic and async behavior.Source: Coding guidelines
71-147: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSet cache headers on responses via
setCacheHeaders.None of the response paths set caching headers. Beyond the path-instruction requirement, the Twitter/X API v2 enforces tight rate limits, and this endpoint is hit on every dashboard Details view; short-lived response caching would materially reduce upstream calls and the risk of hitting those limits.
As per path instructions: "Use setCacheHeaders utility for setting appropriate caching headers on responses in Next.js API endpoints".
#!/bin/bash # Confirm the setCacheHeaders utility and its expected signature/usage rg -nP -C3 'export\s+(function|const)\s+setCacheHeaders' rg -nP -C2 'setCacheHeaders\s*\(' -g 'src/pages/api/**'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/api/twitter/tweet-stats.ts` around lines 71 - 147, The Twitter stats API responses in tweet-stats should use setCacheHeaders instead of returning uncached JSON from each branch. Update the main success path and every fallback path in the tweet stats handler to call setCacheHeaders on the Next.js response before res.status(...).json(...), using the existing setCacheHeaders utility with an appropriate short-lived cache policy. Make sure the change is applied consistently in the tweet stats endpoint’s fetch success, non-ok response, missing metrics, and early-return paths so all responses share the same caching behavior.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/pages/api/twitter/tweet-stats.ts`:
- Line 47: The top-level module function handler is missing an explicit return
type, so add Promise<void> to its declaration. Update the handler signature in
the NextApiRequestWithUser / NextApiResponse API route so it clearly declares
its return type while preserving the existing logic and async behavior.
- Around line 71-147: The Twitter stats API responses in tweet-stats should use
setCacheHeaders instead of returning uncached JSON from each branch. Update the
main success path and every fallback path in the tweet stats handler to call
setCacheHeaders on the Next.js response before res.status(...).json(...), using
the existing setCacheHeaders utility with an appropriate short-lived cache
policy. Make sure the change is applied consistently in the tweet stats
endpoint’s fetch success, non-ok response, missing metrics, and early-return
paths so all responses share the same caching behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19028a97-399b-4fe5-96f7-9c743ae1e19f
📒 Files selected for processing (4)
src/features/sponsor-dashboard/components/Submissions/Details.tsxsrc/features/sponsor-dashboard/components/Submissions/TweetStats.tsxsrc/pages/api/sponsor-dashboard/listings/verify-external-payment.tssrc/pages/api/twitter/tweet-stats.ts
✅ Files skipped from review due to trivial changes (1)
- src/pages/api/sponsor-dashboard/listings/verify-external-payment.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/features/sponsor-dashboard/components/Submissions/Details.tsx
- src/features/sponsor-dashboard/components/Submissions/TweetStats.tsx
…sor dashboard
What does this PR do?
Where should the reviewer start?
How should this be manually tested?
Any background context you want to provide?
What are the relevant issues?
Screenshots (if appropriate)
Summary by CodeRabbit