-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Adds test env vars #2911
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
base: main
Are you sure you want to change the base?
Adds test env vars #2911
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds environment-driven test toggles and domain normalization for the web app: optional Vercel domain addition, optional bot-detection bypass, optional rate-limit bypass, and TEST_DOMAINS parsing; exposes those variables in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as Domains API
participant Vercel as Vercel API
Client->>API: POST /api/domains
alt TEST_SKIP_ADD_DOMAIN !== "true"
API->>Vercel: addDomainToVercel(domain)
Vercel-->>API: response / error
API-->>Client: Result based on Vercel response
else Skip enabled
API-->>Client: Result without calling Vercel
end
sequenceDiagram
autonumber
actor User
participant Srv as Server
participant Bot as Bot Detector
participant RL as Rate Limiter
participant TB as Analytics (recordClick)
User->>Srv: Request
opt Link middleware
alt TEST_SKIP_BOT_DETECTION === "true"
Srv->>Srv: isBot = false
else
Srv->>Bot: detectBot(req)
Bot-->>Srv: isBot flag
end
end
User->>TB: recordClick(...)
alt TEST_SKIP_RATE_LIMIT === "true" or skipRateLimitProp
TB->>TB: Bypass rate limit
else
TB->>RL: Check rate
RL-->>TB: Allow / Deny
end
TB-->>User: Recorded / Skipped
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/lib/middleware/utils/parse.ts (1)
19-30
: Consider trimming test domains to handle whitespace.If
TEST_DOMAINS
contains spaces around commas (e.g.,"click.localhost:8888, other.localhost:8888"
), the current implementation won't match domains correctly. Consider trimming each entry after splitting.Apply this diff:
} else if (process.env.TEST_DOMAINS) { - const testDomains = process.env.TEST_DOMAINS.split(","); + const testDomains = process.env.TEST_DOMAINS.split(",").map(d => d.trim()); // Check if the current domain (with port) matches any test domain, and if so, strip the port const matchedTestDomain = testDomains.find(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/.env.example
(1 hunks)apps/web/app/api/domains/route.ts
(1 hunks)apps/web/lib/middleware/link.ts
(1 hunks)apps/web/lib/middleware/utils/parse.ts
(1 hunks)apps/web/lib/tinybird/record-click.ts
(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/app/api/domains/route.ts (1)
apps/web/lib/api/domains/add-domain-vercel.ts (1)
addDomainToVercel
(5-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
apps/web/lib/middleware/link.ts (1)
300-301
: LGTM! Bot detection bypass is correctly implemented.The conditional bot detection logic properly checks for the exact string
"true"
and falls back to the originaldetectBot(req)
behavior otherwise. This enables testing without bot interference while preserving production behavior.apps/web/app/api/domains/route.ts (1)
139-150
: LGTM! Domain addition skip is correctly implemented.The conditional logic properly gates the Vercel API call and its error handling, allowing tests to bypass external dependencies without affecting production behavior.
apps/web/lib/tinybird/record-click.ts (3)
75-76
: LGTM! Bot detection bypass is consistent.The test-time bot detection bypass matches the implementation in
link.ts
and correctly defaults todetectBot(req)
when the flag is not set.
91-92
: LGTM! Rate limit bypass logic is correct.The implementation properly computes
skipRatelimit
from either the function parameter or the test environment flag, enabling flexible test-time behavior.
38-38
: Parameter name inconsistency detected.Line 38 renames the destructured parameter to
skipRateLimitProp
, but line 52 in the type definition still declares it asskipRatelimit?: boolean
. This creates a mismatch between the parameter name and its type definition.Apply this diff to align the type definition:
webhookIds?: string[]; workspaceId: string | undefined; - skipRatelimit?: boolean; + skipRateLimitProp?: boolean; timestamp?: string; referrer?: string; trigger?: string;Likely an incorrect or invalid review comment.
Adds the following test env vars for local development:
TEST_DOMAINS
TEST_SKIP_ADD_DOMAIN
TEST_SKIP_BOT_DETECTION
TEST_SKIP_RATE_LIMIT
Summary by CodeRabbit
Tests
Chores