Dynamic UTM parameters for partner links - #4245
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR adds shared partner UTM macros, applies resolved templates across link creation and update flows, adds UTM autocomplete suggestions, and moves group UTM synchronization into a paginated background job. ChangesPartner UTM and synchronization
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant PartnerUpdate
participant DispatchGroupUtmSyncForPartner
participant SyncGroupUtmJob
participant Database
participant Redis
PartnerUpdate->>DispatchGroupUtmSyncForPartner: dispatch after partner name change
DispatchGroupUtmSyncForPartner->>SyncGroupUtmJob: dispatch for each group
SyncGroupUtmJob->>Database: load enrollments and links
SyncGroupUtmJob->>Database: batch update resolved URLs and UTM fields
SyncGroupUtmJob->>Redis: expire link cache entries
SyncGroupUtmJob->>SyncGroupUtmJob: dispatch next enrollment page
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/app/(ee)/api/partners/links/route.ts (1)
131-148: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftResolve macros only after determining the effective link key.
Both routes build
utmContextfrom the optional requestkey, but passundefinedtoprocessLinkwhen no key is provided.processLinkthen generates the actual key after{{PARTNER_LINK_KEY}}has already been resolved.
apps/web/app/(ee)/api/partners/links/route.ts#L131-L148: generate or reserve the key before resolving the group UTM template, then use it inutmContextand the link payload.apps/web/app/(ee)/api/partners/links/upsert/route.ts#L201-L219: apply the same effective-key flow in the create branch.🤖 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 `@apps/web/app/`(ee)/api/partners/links/route.ts around lines 131 - 148, Resolve an effective partner link key before processing UTM macros. In apps/web/app/(ee)/api/partners/links/route.ts lines 131-148, generate or reserve the key when the request key is absent, then use that key consistently in utmContext and the processLink payload. Apply the same effective-key flow in the create branch of apps/web/app/(ee)/api/partners/links/upsert/route.ts lines 201-219.apps/web/app/(ee)/api/cron/groups/create-default-links/route.ts (1)
140-189: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftResolve
PARTNER_LINK_KEYafter duplicate-key handling.
generatePartnerLinkappends a suffix when the requested key already exists. Both callers resolve UTM values before this retry. The persisted short-link key can then differ from the URL and UTM-column value for{{PARTNER_LINK_KEY}}.
apps/web/app/(ee)/api/cron/groups/create-default-links/route.ts#L140-L189: resolve UTM values with the finalcurrentKey.apps/web/lib/api/partners/create-partner-default-links.ts#L69-L93: resolve UTM values with the finalcurrentKey.Move this resolution into
generatePartnerLink, or return the final key before constructing the URL and UTM columns. Add a duplicate-key test.🤖 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 `@apps/web/app/`(ee)/api/cron/groups/create-default-links/route.ts around lines 140 - 189, Ensure UTM values use the final partner link key after duplicate-key handling, so {{PARTNER_LINK_KEY}} matches the persisted short-link key. Update generatePartnerLink or both callers to resolve the URL and UTM columns only after the final currentKey is determined; apply this in apps/web/app/(ee)/api/cron/groups/create-default-links/route.ts lines 140-189 and apps/web/lib/api/partners/create-partner-default-links.ts lines 69-93, then add a duplicate-key test covering the suffixed key.apps/web/app/(ee)/api/partner-profile/programs/[programId]/links/route.ts (1)
112-125: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDerive and persist the same fallback key before resolving macros.
When the request omits
key,partnerLinkKeybecomes"".{{PARTNER_LINK_KEY}}then resolves topartner.name, which can differ from the generated short-link key. For example,"John Doe"is not the derived key"john-doe".Use
derivePartnerLinkKeyfor the macro context. Pass that derived key toprocessLinkwhen the request omitskey. Add coverage for a request withoutkey.Proposed fix
+import { derivePartnerLinkKey } from "`@/lib/api/partners/generate-partner-link`"; - const partnerLinkKey = key || ""; + const partnerLinkKey = derivePartnerLinkKey({ + key, + username: partner.username, + name: partner.name, + email: partner.email, + }); const utmContext = { partnerName: partner.name || partnerLinkKey, - partnerLinkKey: partnerLinkKey || partner.name || "", + partnerLinkKey, }; const { link, error, code } = await processLink({ payload: { domain: program.domain, - key: key || undefined, + key: partnerLinkKey,🤖 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 `@apps/web/app/`(ee)/api/partner-profile/programs/[programId]/links/route.ts around lines 112 - 125, Update the link creation flow around partnerLinkKey and processLink to derive the fallback key with derivePartnerLinkKey when the request omits key, use that same derived value for the UTM macro context’s partnerLinkKey, and pass it to processLink so macro resolution and persisted short-link creation remain consistent. Add coverage for a request without key.
🧹 Nitpick comments (3)
packages/ui/src/utm-builder.tsx (2)
97-100: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant match clause.
value.includes(q)already matches everything thatvalue.slice(2).startsWith(q)matches, becauseslice(2)is a substring ofvalue. The second clause never changes the result.♻️ Proposed refactor
- return suggestions.filter((s) => { - const value = s.value.toLowerCase(); - return value.includes(q) || value.slice(2).startsWith(q); - }); + return suggestions.filter((s) => s.value.toLowerCase().includes(q));🤖 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 `@packages/ui/src/utm-builder.tsx` around lines 97 - 100, Remove the redundant value.slice(2).startsWith(q) condition from the suggestions filter in the UTM builder, leaving the filter based solely on value.includes(q).
258-330: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd combobox semantics to the autocomplete.
The input and the portaled menu carry no ARIA relationship. Screen readers do not announce that the list opened, and they do not announce the highlighted option. The
<button>elements also sit at the end ofdocument.body, so Tab moves focus out of the form into the menu.Wire the pattern explicitly:
- On the input:
role="combobox",aria-expanded,aria-controls,aria-autocomplete="list",aria-activedescendantpointing at the highlighted option id.- On the menu:
role="listbox"with an id, androle="option"plusaria-selectedon each item.- On each item:
tabIndex={-1}, so keyboard focus stays in the input.♿ Proposed wiring
<input type="text" id={id} ref={setInputRef} + role="combobox" + aria-expanded={menuOpen && filtered.length > 0} + aria-controls={`${id}-listbox`} + aria-autocomplete="list" + aria-activedescendant={ + menuOpen && filtered.length > 0 + ? `${id}-option-${highlightedIndex}` + : undefined + } placeholder={placeholder}<div style={menuStyle} + id={`${id}-listbox`} + role="listbox" className="border-border-subtle flex flex-col rounded-lg border bg-white p-1 shadow-sm" > {filtered.map((suggestion, index) => ( <button key={suggestion.value} type="button" + id={`${id}-option-${index}`} + role="option" + aria-selected={highlightedIndex === index} + tabIndex={-1} onMouseDown={(e) => e.preventDefault()}🤖 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 `@packages/ui/src/utm-builder.tsx` around lines 258 - 330, Update the autocomplete input and portaled suggestion menu to use explicit combobox semantics: add a stable listbox id and connect it through the input’s role, aria-expanded, aria-controls, aria-autocomplete, and aria-activedescendant attributes. Mark the menu as role="listbox", each suggestion button as role="option" with aria-selected tied to highlightedIndex and a unique option id, and set each item’s tabIndex to -1 so focus remains on the input.apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/links/group-link-settings.tsx (1)
282-286: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the helper text from
PARTNER_MACROS.The prose repeats the macro names that are already imported. If the shared module gains or renames a macro, this text becomes wrong while the autocomplete stays correct. Render the list from the same source.
♻️ Proposed refactor
- <p className="text-content-muted text-xs"> - Dynamic values:{" "} - <code className="font-mono">{"{{PARTNER_NAME}}"}</code>,{" "} - <code className="font-mono">{"{{PARTNER_LINK_KEY}}"}</code> - </p> + <p className="text-content-muted text-xs"> + Dynamic values:{" "} + {PARTNER_MACROS.map((m, idx) => ( + <Fragment key={m.macro}> + {idx > 0 && ", "} + <code className="font-mono">{m.macro}</code> + </Fragment> + ))} + </p>Add the
Fragmentimport:import { Fragment, useState } from "react";🤖 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 `@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/links/group-link-settings.tsx around lines 282 - 286, Update the helper text near the group-link settings UI to derive its macro list from the imported PARTNER_MACROS source instead of hardcoded names. Render each macro name from that collection with the existing styling and punctuation, adding the React Fragment import if needed for list rendering.
🤖 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 `@packages/ui/src/utm-builder.tsx`:
- Around line 176-184: Update insertSuggestion to consume the full open or
completed macro when replacing a suggestion: use the detected token’s end
position, including the closing }} when present, instead of always slicing the
tail from caret. Preserve the existing fallback behavior for values without a
detected token, and ensure nextCaret is based on the replacement start plus
suggestion.value.length.
---
Outside diff comments:
In `@apps/web/app/`(ee)/api/cron/groups/create-default-links/route.ts:
- Around line 140-189: Ensure UTM values use the final partner link key after
duplicate-key handling, so {{PARTNER_LINK_KEY}} matches the persisted short-link
key. Update generatePartnerLink or both callers to resolve the URL and UTM
columns only after the final currentKey is determined; apply this in
apps/web/app/(ee)/api/cron/groups/create-default-links/route.ts lines 140-189
and apps/web/lib/api/partners/create-partner-default-links.ts lines 69-93, then
add a duplicate-key test covering the suffixed key.
In `@apps/web/app/`(ee)/api/partner-profile/programs/[programId]/links/route.ts:
- Around line 112-125: Update the link creation flow around partnerLinkKey and
processLink to derive the fallback key with derivePartnerLinkKey when the
request omits key, use that same derived value for the UTM macro context’s
partnerLinkKey, and pass it to processLink so macro resolution and persisted
short-link creation remain consistent. Add coverage for a request without key.
In `@apps/web/app/`(ee)/api/partners/links/route.ts:
- Around line 131-148: Resolve an effective partner link key before processing
UTM macros. In apps/web/app/(ee)/api/partners/links/route.ts lines 131-148,
generate or reserve the key when the request key is absent, then use that key
consistently in utmContext and the processLink payload. Apply the same
effective-key flow in the create branch of
apps/web/app/(ee)/api/partners/links/upsert/route.ts lines 201-219.
---
Nitpick comments:
In
`@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/links/group-link-settings.tsx:
- Around line 282-286: Update the helper text near the group-link settings UI to
derive its macro list from the imported PARTNER_MACROS source instead of
hardcoded names. Render each macro name from that collection with the existing
styling and punctuation, adding the React Fragment import if needed for list
rendering.
In `@packages/ui/src/utm-builder.tsx`:
- Around line 97-100: Remove the redundant value.slice(2).startsWith(q)
condition from the suggestions filter in the UTM builder, leaving the filter
based solely on value.includes(q).
- Around line 258-330: Update the autocomplete input and portaled suggestion
menu to use explicit combobox semantics: add a stable listbox id and connect it
through the input’s role, aria-expanded, aria-controls, aria-autocomplete, and
aria-activedescendant attributes. Mark the menu as role="listbox", each
suggestion button as role="option" with aria-selected tied to highlightedIndex
and a unique option id, and set each item’s tabIndex to -1 so focus remains on
the input.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 808a48a3-ccf8-46ae-9ce9-0546a44ec1a7
📒 Files selected for processing (25)
apps/web/app/(ee)/api/cron/groups/create-default-links/route.tsapps/web/app/(ee)/api/cron/groups/remap-default-links/route.tsapps/web/app/(ee)/api/cron/groups/sync-utm/route.tsapps/web/app/(ee)/api/cron/groups/update-default-links/route.tsapps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/links/[linkId]/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/links/route.tsapps/web/app/(ee)/api/partners/links/route.tsapps/web/app/(ee)/api/partners/links/upsert/route.tsapps/web/app/api/utm/[id]/route.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/links/group-link-settings.tsxapps/web/lib/api/partners/create-partner-default-links.tsapps/web/lib/api/utm/extract-and-resolve-utm-params.tsapps/web/lib/integrations/appsflyer/apply-parameters.tsapps/web/lib/integrations/appsflyer/constants.tsapps/web/lib/integrations/appsflyer/macro-template.tsapps/web/lib/integrations/appsflyer/schema.tsapps/web/lib/integrations/appsflyer/ui/settings.tsxapps/web/lib/jobs/handlers/sync-group-utm-job.tsapps/web/lib/jobs/registry.tsapps/web/lib/partners/macros.tsapps/web/lib/zod/schemas/utm.tsapps/web/tests/misc/partner-macros.test.tsapps/web/ui/modals/add-partner-link-modal.tsxpackages/ui/src/utm-builder.tsx
💤 Files with no reviewable changes (2)
- apps/web/lib/integrations/appsflyer/constants.ts
- apps/web/lib/integrations/appsflyer/macro-template.ts
Summary by CodeRabbit
New Features
Improvements
Validation