fix: expose campaign unsubscribe variable#433
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughCampaign personalization now documents Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/src/lib/constants/campaign.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. apps/web/src/lib/constants/campaign.unit.test.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. 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 |
Deploying usesend with
|
| Latest commit: |
8eadf67
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://27508f48.usesend.pages.dev |
| Branch Preview URL: | https://codex-document-campaign-unsu.usesend.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/web/src/lib/constants/campaign.ts`:
- Around line 18-20: The helper getCampaignUnsubscribeVariableValues must return
both canonical and legacy unsubscribe variable keys, each mapped to
unsubscribeUrl, while keeping only the canonical key in editor autocomplete. In
apps/web/src/lib/constants/campaign.ts lines 18-20, update the helper
accordingly; in apps/web/src/server/service/campaign-service.ts lines 119-143,
continue spreading its result into variableValues and add a regression test
covering the legacy variable in non-link structured content.
🪄 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: 3d8e3e71-58f3-4762-972e-e1ea899db58c
📒 Files selected for processing (3)
apps/docs/guides/campaign-personalization.mdxapps/web/src/lib/constants/campaign.tsapps/web/src/server/service/campaign-service.ts
💤 Files with no reviewable changes (1)
- apps/docs/guides/campaign-personalization.mdx
| export function getCampaignUnsubscribeVariableValues(unsubscribeUrl: string) { | ||
| return { [CAMPAIGN_UNSUBSCRIBE_VARIABLE]: unsubscribeUrl }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Complete legacy unsubscribe compatibility in structured rendering.
The shared variable-value contract exposes only usesend_unsubscribe_url, while the service claims to preserve unsend_unsubscribe_url. Return both aliases from the helper; keep only the canonical name in editor autocomplete.
apps/web/src/lib/constants/campaign.ts#L18-L20: return both unsubscribe variable keys mapped tounsubscribeUrl.apps/web/src/server/service/campaign-service.ts#L119-L143: continue spreading the helper result intovariableValuesand add a regression test for legacy variables in non-link structured content.
📍 Affects 2 files
apps/web/src/lib/constants/campaign.ts#L18-L20(this comment)apps/web/src/server/service/campaign-service.ts#L119-L143
🤖 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/src/lib/constants/campaign.ts` around lines 18 - 20, The helper
getCampaignUnsubscribeVariableValues must return both canonical and legacy
unsubscribe variable keys, each mapped to unsubscribeUrl, while keeping only the
canonical key in editor autocomplete. In apps/web/src/lib/constants/campaign.ts
lines 18-20, update the helper accordingly; in
apps/web/src/server/service/campaign-service.ts lines 119-143, continue
spreading its result into variableValues and add a regression test covering the
legacy variable in non-link structured content.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/lib/constants/campaign.unit.test.ts (1)
10-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExport and reuse the legacy variable constant.
Instead of hardcoding
"unsend_unsubscribe_url", consider exportingLEGACY_CAMPAIGN_UNSUBSCRIBE_VARIABLEfromcampaign.tsand using it here (and on line 60). This removes magic strings and ensures the test stays aligned with the implementation.🛠️ Proposed refactor
In
apps/web/src/lib/constants/campaign.ts:-const LEGACY_CAMPAIGN_UNSUBSCRIBE_VARIABLE = "unsend_unsubscribe_url"; +export const LEGACY_CAMPAIGN_UNSUBSCRIBE_VARIABLE = "unsend_unsubscribe_url";In this file:
-import { - CAMPAIGN_UNSUBSCRIBE_VARIABLE, - getCampaignEditorVariables, - getCampaignUnsubscribeVariableValues, -} from "~/lib/constants/campaign"; +import { + CAMPAIGN_UNSUBSCRIBE_VARIABLE, + LEGACY_CAMPAIGN_UNSUBSCRIBE_VARIABLE, + getCampaignEditorVariables, + getCampaignUnsubscribeVariableValues, +} from "~/lib/constants/campaign"; ... it("includes the canonical unsubscribe variable", () => { const variables = getCampaignEditorVariables(); expect(variables).toContain(CAMPAIGN_UNSUBSCRIBE_VARIABLE); - expect(variables).not.toContain("unsend_unsubscribe_url"); + expect(variables).not.toContain(LEGACY_CAMPAIGN_UNSUBSCRIBE_VARIABLE); }); ... it("renders a legacy structured unsubscribe variable with the recipient URL", async () => { - const legacyVariable = "unsend_unsubscribe_url"; + const legacyVariable = LEGACY_CAMPAIGN_UNSUBSCRIBE_VARIABLE; const renderer = new EmailRenderer({🤖 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/src/lib/constants/campaign.unit.test.ts` around lines 10 - 15, Export LEGACY_CAMPAIGN_UNSUBSCRIBE_VARIABLE from campaign.ts, then import and reuse it in campaign.unit.test.ts for both legacy-variable assertions, replacing the hardcoded "unsend_unsubscribe_url" strings while preserving the existing expectations.
🤖 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 `@apps/web/src/lib/constants/campaign.unit.test.ts`:
- Around line 10-15: Export LEGACY_CAMPAIGN_UNSUBSCRIBE_VARIABLE from
campaign.ts, then import and reuse it in campaign.unit.test.ts for both
legacy-variable assertions, replacing the hardcoded "unsend_unsubscribe_url"
strings while preserving the existing expectations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 04b604a7-a0b3-4627-a3fd-5e50a0094319
📒 Files selected for processing (2)
apps/web/src/lib/constants/campaign.tsapps/web/src/lib/constants/campaign.unit.test.ts
Summary
{{usesend_unsubscribe_url}}as the required, recipient-specific campaign unsubscribe variableImpact
Campaign authors can discover the required unsubscribe variable in both the guide and editor, and autocomplete selections render to the correct per-recipient URL.
Screenshots
Not included: the existing autocomplete menu only gains one text suggestion; there are no layout or styling changes.
Migrations
None.
Verification
pnpm exec vitest run -c vitest.unit.config.ts src/lib/constants/campaign.unit.test.ts src/server/service/campaign-service.unit.test.ts(8 tests passed)pnpm --filter=web typecheckCloses #427
Summary by CodeRabbit
{{usesend_unsubscribe_url}}.