Skip to content

Commit a7e5f86

Browse files
authored
Merge pull request dubinc#2326 from dubinc/shouldPassClickId
Update `shouldPassClickId` implementation
2 parents df24474 + 5c52f34 commit a7e5f86

5 files changed

Lines changed: 33 additions & 24 deletions

File tree

apps/web/app/api/track/click/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ export const POST = withAxiom(async (req: AxiomRequest) => {
120120
workspaceId: cachedLink.projectId,
121121
skipRatelimit: true,
122122
...(referrer && { referrer }),
123-
trackConversion: cachedLink.trackConversion,
123+
shouldPassClickId: true,
124124
});
125125
}
126126
})(),

apps/web/app/api/track/visit/route.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ export const POST = withAxiom(async (req: AxiomRequest) => {
9595
workspaceId: cachedLink.projectId,
9696
skipRatelimit: true,
9797
...(referrer && { referrer }),
98-
trackConversion: cachedLink.trackConversion,
9998
});
10099
}
101100
})(),

apps/web/lib/middleware/link.ts

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,11 @@ export default async function LinkMiddleware(
167167

168168
const url = testUrl || cachedLink.url;
169169

170+
// we only pass the clickId if:
171+
// - trackConversion is enabled
172+
// - it's a partner link
173+
const shouldPassClickId = trackConversion || isPartnerLink;
174+
170175
// by default, we only index default dub domain links (e.g. dub.sh)
171176
// everything else is not indexed by default, unless the user has explicitly set it to be indexed
172177
const shouldIndex = isDubDomain(domain) || doIndex === true;
@@ -244,8 +249,8 @@ export default async function LinkMiddleware(
244249
const cookieStore = cookies();
245250
let clickId = cookieStore.get(dubIdCookieName)?.value;
246251
if (!clickId) {
247-
// if trackConversion is enabled, check if clickId is cached in Redis
248-
if (trackConversion) {
252+
// if we need to pass the clickId, check if clickId is cached in Redis
253+
if (shouldPassClickId) {
249254
const ip = process.env.VERCEL === "1" ? ipAddress(req) : LOCALHOST_IP;
250255

251256
clickId = (await clickCache.get({ domain, key, ip })) || undefined;
@@ -276,7 +281,7 @@ export default async function LinkMiddleware(
276281
url,
277282
webhookIds,
278283
workspaceId,
279-
trackConversion,
284+
shouldPassClickId,
280285
}),
281286
);
282287

@@ -297,12 +302,6 @@ export default async function LinkMiddleware(
297302
const { country } =
298303
process.env.VERCEL === "1" && req.geo ? req.geo : LOCALHOST_GEO_DATA;
299304

300-
// we only pass the clickId if:
301-
// - trackConversion is enabled
302-
// - not a partner link (TODO: add this later) !isPartnerLink
303-
// - there is a clickId
304-
const shouldPassClickId = trackConversion && clickId;
305-
306305
// rewrite to proxy page (/proxy/[domain]/[key]) if it's a bot and proxy is enabled
307306
if (isBot && proxy) {
308307
return createResponseWithCookies(
@@ -330,7 +329,7 @@ export default async function LinkMiddleware(
330329
url,
331330
webhookIds,
332331
workspaceId,
333-
trackConversion,
332+
shouldPassClickId,
334333
}),
335334
);
336335

@@ -368,7 +367,7 @@ export default async function LinkMiddleware(
368367
url,
369368
webhookIds,
370369
workspaceId,
371-
trackConversion,
370+
shouldPassClickId,
372371
}),
373372
);
374373

@@ -408,7 +407,7 @@ export default async function LinkMiddleware(
408407
url: ios,
409408
webhookIds,
410409
workspaceId,
411-
trackConversion,
410+
shouldPassClickId,
412411
}),
413412
);
414413

@@ -442,7 +441,7 @@ export default async function LinkMiddleware(
442441
url: android,
443442
webhookIds,
444443
workspaceId,
445-
trackConversion,
444+
shouldPassClickId,
446445
}),
447446
);
448447

@@ -476,7 +475,7 @@ export default async function LinkMiddleware(
476475
url: geo[country],
477476
webhookIds,
478477
workspaceId,
479-
trackConversion,
478+
shouldPassClickId,
480479
}),
481480
);
482481

@@ -510,7 +509,7 @@ export default async function LinkMiddleware(
510509
url,
511510
webhookIds,
512511
workspaceId,
513-
trackConversion,
512+
shouldPassClickId,
514513
}),
515514
);
516515

@@ -521,7 +520,7 @@ export default async function LinkMiddleware(
521520
headers: new Headers({
522521
destination: getFinalUrl(url, {
523522
req,
524-
clickId: trackConversion ? clickId : undefined,
523+
clickId: shouldPassClickId ? clickId : undefined,
525524
}),
526525
}),
527526
},

apps/web/lib/tinybird/record-click.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ export async function recordClick({
3737
skipRatelimit,
3838
timestamp,
3939
referrer,
40-
trackConversion,
40+
shouldPassClickId,
4141
}: {
4242
req: Request;
4343
clickId?: string;
@@ -50,7 +50,7 @@ export async function recordClick({
5050
skipRatelimit?: boolean;
5151
timestamp?: string;
5252
referrer?: string;
53-
trackConversion?: boolean;
53+
shouldPassClickId?: boolean;
5454
}) {
5555
if (!clickId) {
5656
return null;
@@ -165,7 +165,7 @@ export async function recordClick({
165165

166166
// cache the click data for 5 mins
167167
// we're doing this because ingested click events are not available immediately in Tinybird
168-
trackConversion &&
168+
shouldPassClickId &&
169169
redis.set(`clickCache:${clickId}`, clickData, {
170170
ex: 60 * 5,
171171
}),

apps/web/tests/redirects/index.test.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,20 @@ describe.runIf(env.CI)("Link Redirects", async () => {
5555
expect(response.status).toBe(302);
5656
});
5757

58+
test("with dub_id and via", async () => {
59+
const response = await fetch(`${h.baseUrl}/track-test`, {
60+
...fetchOptions,
61+
headers: {},
62+
});
63+
64+
// the location should contain `?dub_id=` query param
65+
expect(response.headers.get("location")).toMatch(/dub_id=[a-zA-Z0-9]+/);
66+
// the location should contain `?via=track-test` query param
67+
expect(response.headers.get("location")).toMatch(/via=track-test/);
68+
expect(response.headers.get("x-powered-by")).toBe(poweredBy);
69+
expect(response.status).toBe(302);
70+
});
71+
5872
test("with dub_client_reference_id", async () => {
5973
const response = await fetch(`${h.baseUrl}/client_reference_id`, {
6074
...fetchOptions,
@@ -167,7 +181,4 @@ describe.runIf(env.CI)("Link Redirects", async () => {
167181
expect(response.headers.get("x-powered-by")).toBe(poweredBy);
168182
expect(response.status).toBe(302);
169183
});
170-
171-
// DUMMY test to record a hit on track-test
172-
await fetch(`${h.baseUrl}/track-test`);
173184
});

0 commit comments

Comments
 (0)