Leverage clickIdCache to reduce load on Tinybird #2981
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
WalkthroughThe PR refactors click data retrieval infrastructure by converting Changes
Sequence DiagramsequenceDiagram
participant Caller as API Route/Function
participant GetClickEvent as getClickEvent()
participant Redis as Redis Cache
participant Tinybird as Tinybird Pipe
Caller->>GetClickEvent: getClickEvent({ clickId })
GetClickEvent->>Redis: redis.get(clickIdCache:clickId)
alt Cache Hit
Redis-->>GetClickEvent: ClickEventTB (cached)
GetClickEvent->>GetClickEvent: Transform fields<br/>(timestamp, qr, bot)
GetClickEvent-->>Caller: clickData (transformed)
else Cache Miss
GetClickEvent->>Tinybird: getClickEventTB({ clickId })
Tinybird-->>GetClickEvent: { data: [...] }
GetClickEvent->>GetClickEvent: Extract data[0]
GetClickEvent-->>Caller: clickData (first item)
else Error
Tinybird-->>GetClickEvent: Error
GetClickEvent->>GetClickEvent: Log error
GetClickEvent-->>Caller: null
end
Caller->>Caller: Check if clickData exists<br/>Throw not_found if falsy
Caller->>Caller: Use clickData properties<br/>(link_id, country, timestamp)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The refactor follows a consistent, homogeneous pattern across multiple files (click data access simplification), reducing cognitive overhead. However, the scope spans 11 files with varying context (API routes, conversion tracking, scripts, utilities), and the core function signature change requires verification that all call sites correctly consume the new return type. The addition of Redis caching logic adds a layer of complexity to verify. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/lib/api/conversions/track-sale.ts (1)
62-62: Remove unused outer variable declaration.The
clickDatavariable declared at Line 62 is shadowed by the declaration at Line 154 and never used. Consider removing the outer declaration to eliminate confusion.Apply this diff:
let existingCustomer: Customer | null = null; let newCustomer: Customer | null = null; - let clickData: ClickEventTB | null = null; let leadEventData: LeadEventTB | null = null;Also applies to: 154-156
apps/web/lib/tinybird/get-click-event.ts (1)
15-15: Add explicit return type annotation.For better type safety and clarity, add an explicit return type to the function signature.
Apply this diff:
-export const getClickEvent = async ({ clickId }: { clickId: string }) => { +export const getClickEvent = async ({ clickId }: { clickId: string }): Promise<ClickEventTB | null> => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/web/app/(ee)/api/shopify/pixel/route.ts(1 hunks)apps/web/app/(ee)/api/stripe/integration/webhook/checkout-session-completed.ts(1 hunks)apps/web/app/(ee)/api/stripe/integration/webhook/utils/create-new-customer.ts(1 hunks)apps/web/lib/api/conversions/track-lead.ts(2 hunks)apps/web/lib/api/conversions/track-sale.ts(1 hunks)apps/web/lib/integrations/shopify/create-lead.ts(2 hunks)apps/web/lib/tinybird/get-click-event.ts(1 hunks)apps/web/lib/tinybird/record-click.ts(1 hunks)apps/web/lib/webhook/transform.ts(1 hunks)apps/web/scripts/migrations/backfill-attribution.ts(1 hunks)apps/web/scripts/tinybird/update-click-event.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-02T22:46:22.739Z
Learnt from: steven-tey
PR: dubinc/dub#2924
File: apps/web/lib/api/conversions/track-lead.ts:7-7
Timestamp: 2025-10-02T22:46:22.739Z
Learning: In apps/web/lib/api/conversions/track-lead.ts, lead events are cached in Redis for 5 minutes (keys: `leadCache:${customer.id}` and `leadCache:${customer.id}:${stringifiedEventName}`) to provide immediate data availability while Tinybird ingestion happens asynchronously. This caching pattern allows for async-only recording without breaking "wait" mode semantics.
Applied to files:
apps/web/lib/api/conversions/track-lead.tsapps/web/lib/tinybird/record-click.ts
🧬 Code graph analysis (9)
apps/web/scripts/migrations/backfill-attribution.ts (1)
apps/web/lib/tinybird/get-click-event.ts (1)
getClickEvent(15-41)
apps/web/lib/integrations/shopify/create-lead.ts (2)
apps/web/lib/tinybird/get-click-event.ts (1)
getClickEvent(15-41)apps/web/lib/api/errors.ts (1)
DubApiError(75-92)
apps/web/lib/api/conversions/track-lead.ts (1)
apps/web/lib/tinybird/get-click-event.ts (1)
getClickEvent(15-41)
apps/web/scripts/tinybird/update-click-event.ts (1)
apps/web/lib/tinybird/get-click-event.ts (1)
getClickEvent(15-41)
apps/web/lib/api/conversions/track-sale.ts (1)
apps/web/lib/tinybird/get-click-event.ts (1)
getClickEvent(15-41)
apps/web/app/(ee)/api/stripe/integration/webhook/checkout-session-completed.ts (1)
apps/web/lib/tinybird/get-click-event.ts (1)
getClickEvent(15-41)
apps/web/lib/tinybird/record-click.ts (1)
apps/web/lib/upstash/redis.ts (1)
redis(4-7)
apps/web/lib/tinybird/get-click-event.ts (3)
apps/web/lib/tinybird/client.ts (1)
tb(3-6)apps/web/lib/upstash/redis.ts (1)
redis(4-7)apps/web/lib/types.ts (1)
ClickEventTB(559-559)
apps/web/app/(ee)/api/stripe/integration/webhook/utils/create-new-customer.ts (1)
apps/web/lib/tinybird/get-click-event.ts (1)
getClickEvent(15-41)
⏰ 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 (13)
apps/web/lib/tinybird/record-click.ts (1)
170-176: Cache expiry extension looks good.The 1-day cache duration aligns with the PR objective to reduce load on Tinybird. The comment and code are consistent.
apps/web/app/(ee)/api/shopify/pixel/route.ts (1)
36-42: LGTM!The validation logic correctly handles the updated
getClickEventreturn type, which now returnsclickDatadirectly instead of a nested structure.apps/web/app/(ee)/api/stripe/integration/webhook/checkout-session-completed.ts (1)
72-76: LGTM!The updated call to
getClickEventcorrectly uses the simplified return value, and the null check appropriately handles missing click data.apps/web/app/(ee)/api/stripe/integration/webhook/utils/create-new-customer.ts (1)
29-35: LGTM!The variable rename from
clickEventtoclickDataand the simplified null check correctly reflect the updatedgetClickEventreturn type.apps/web/lib/integrations/shopify/create-lead.ts (1)
38-47: LGTM! Good addition of error handling.The updated code correctly uses the simplified
clickDatareturn value and adds proper error handling withDubApiErrorwhen click data is not found, improving robustness over the previous implementation.apps/web/scripts/migrations/backfill-attribution.ts (2)
39-46: LGTM!The variable rename and simplified null check correctly reflect the updated
getClickEventreturn type.
65-66: LGTM!Field access correctly updated to use
clickDataproperties directly.apps/web/scripts/tinybird/update-click-event.ts (1)
10-14: LGTM!The updated call to
getClickEventcorrectly uses the simplified return value, and the null check appropriately handles missing data.apps/web/lib/api/conversions/track-lead.ts (2)
9-9: LGTM: Import cleanup aligns with refactoring.Removing
ClickEventTBfrom imports is correct since the file no longer needs to reference this type directly—getClickEventnow returns the typed data.
107-115: LGTM: Simplified click data retrieval.The refactoring successfully eliminates nested data extraction and cache fallback logic. The direct error handling is clearer and maintains the same behavior.
apps/web/lib/api/conversions/track-sale.ts (1)
154-163: LGTM: Consistent refactoring pattern.The simplified click data retrieval aligns with the changes in track-lead.ts and correctly handles the null case.
apps/web/lib/tinybird/get-click-event.ts (2)
30-40: LGTM: Robust error handling.The error handling properly logs failures with context and returns null, allowing consumers to handle missing data gracefully.
21-33: Apply consistent data transformation to both cache and Tinybird paths.The cached data undergoes transformation (lines 22-27), converting
timestampformat andqr/botto explicit numbers, but the Tinybird data is returned unmodified (line 33). This creates inconsistent output formats depending on the data source.The schema at
apps/web/lib/zod/schemas/clicks.tsdefinestimestamp,qr, andbot, but the cache path explicitly transforms the timestamp (replacing "T" and "Z") while the Tinybird fallback bypasses this transformation. Ensure both paths return data in the same format by either:
- Applying the same transformation to the Tinybird response before returning it, or
- Documenting that Tinybird already returns data in the transformed format
| // transformLink -> add shortLink, qrCode, workspaceId, etc. | ||
| link: transformLink(lead.link as ExpandedLink), | ||
| metadata: lead.metadata ?? null, | ||
| metadata: lead.metadata || null, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the semantic change from ?? to ||.
The change from nullish coalescing (??) to logical OR (||) means that falsy values like "", 0, and false will now be coerced to null, not just null and undefined. This could impact downstream consumers if they expect to distinguish between empty strings and null.
Additionally, Line 106 still uses ?? for sale metadata, creating an inconsistency between lead and sale transformations.
Consider:
- Whether this semantic change is intentional
- Whether lead and sale metadata should be handled consistently
🏁 Script executed:
#!/bin/bash
# Search for usages of transformLeadEventData and transformSaleEventData to verify impact
rg -nP --type=ts -C3 'transformLeadEventData|transformSaleEventData' -g '!node_modules' -g '!*.test.ts'Length of output: 12052
I'll examine the transform.ts file to verify the exact code and operators being used.
---
🏁 Script executed:
```shell
#!/bin/bash
# Extract the transformLeadEventData and transformSaleEventData functions
sed -n '60,110p' apps/web/lib/webhook/transform.ts
Length of output: 1407
I need to check the schema definitions to understand the expected behavior for metadata fields.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and examine the webhook event schemas
rg -nP --type=ts 'leadWebhookEventSchema|saleWebhookEventSchema' -A 20 apps/web/lib/webhook/transform.ts
Length of output: 1910
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the schemas file in the webhook directory
fd -e ts -e js 'schema' apps/web/lib/webhook/ --maxdepth 2
Length of output: 31
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine the schemas file to see metadata field definitions
cat apps/web/lib/webhook/schemas.ts
Length of output: 3745
Fix the semantic inconsistency between lead and sale metadata handling.
The operators differ between the two functions: line 77 uses || (logical OR) while line 106 uses ?? (nullish coalescing). Given that both schemas define metadata identically as z.record(z.unknown()).nullable(), this inconsistency creates asymmetric behavior. With ||, any falsy value (false, 0, "") is coerced to null; with ??, only null and undefined are coerced. This could cause schema validation failures or inconsistent data handling in sale webhooks if metadata contains falsy non-null values. Both should use the same operator for consistency.
🤖 Prompt for AI Agents
In apps/web/lib/webhook/transform.ts around line 77, the lead metadata uses the
logical OR operator which coerces any falsy value to null; to match the sale
handling at line 106 and the schema z.record(...).nullable(), change the
assignment to use the nullish coalescing operator so metadata is set to
lead.metadata ?? null, ensuring only null/undefined become null and falsy
non-null values are preserved.
1 participant
Summary by CodeRabbit
Release Notes