Skip to content

fix: trim whitespace from contact variable fallbacks - #457

Open
sarmah-rup wants to merge 1 commit into
usesend:mainfrom
sarmah-rup:fix/trim-variable-fallback
Open

sarmah-rup wants to merge 1 commit into
usesend:mainfrom
sarmah-rup:fix/trim-variable-fallback

Conversation

@sarmah-rup

@sarmah-rup sarmah-rup commented Sep 29, 2026 •

Copy link
Copy Markdown

Bug

A contact variable with a padded fallback keeps the padding. With an empty firstName, Hi {{ firstName, fallback= there }}, welcome! renders as Hi there , welcome!, with a stray space before and after the fallback. The same helper renders campaign subjects, API-created campaign HTML and double opt-in subjects.

Root cause

CONTACT_VARIABLE_REGEX captures the fallback with fallback=([^}]+), followed by \s*\}\}. The greedy capture eats the spaces before }} (and any after =), so the trailing \s* matches nothing, and replaceContactVariables returns the raw capture.

Solution

Trim the fallback before returning it. Variable names were already whitespace-tolerant through the regex, and unpadded fallbacks ({{firstName,fallback=there}}, the form in the docs) render the same as before.

Closes #456

Testing

  • Added a case to contact-variable-replacement.unit.test.ts. It fails on main with expected 'Hi there , welcome!' to be 'Hi there, welcome!' and passes with the fix.
  • pnpm test:unit in apps/web: 21 files, 127 tests pass. pnpm test: 31 files, 157 tests pass.
  • Prettier and ESLint are clean on the touched files.

Summary by cubic

Fixes contact variable fallbacks keeping whitespace padding, so Hi {{ firstName, fallback= there }}, welcome! renders as Hi there, welcome! without stray spaces.

Bug Fixes

  • Trims the fallback value captured by the regex before returning it; unpadded fallbacks like {{firstName,fallback=there}} are unaffected.
  • Adds a regression test that fails on main and passes with the fix.

Written for commit f1332ef. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Whitespace around fallback values in contact templates is now trimmed before substitution. Templates without a fallback continue to substitute an empty value.

@vercel

vercel Bot commented Sep 29, 2026

Copy link
Copy Markdown

@sarmah-rup is attempting to deploy a commit to the kmkoushik's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d107bb22-15ce-4637-b7d0-ebd6961b1061

📥 Commits

Reviewing files that changed from the base of the PR and between bcf7e07 and f1332ef.

📒 Files selected for processing (2)
  • apps/web/src/server/utils/contact-variable-replacement.ts
  • apps/web/src/server/utils/contact-variable-replacement.unit.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


Walkthrough

Fallback values in contact variable replacements are trimmed before substitution. When no fallback is provided, the replacement remains an empty string. A unit test covers a whitespace-padded fallback.

Priority: ⬇️ Low

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to f1332

The change is limited to trimming absent-contact fallbacks, with a regression test for padded input. No material merge risk was identified.

Architecture Summary

Architecture risk: 🔵 Low · up to f1332

The change affects 1 system.

Changed systems: apps/web

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/web (ui) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/web/src/server/utils/contact-variable-replacement.ts: Fallback values are trimmed before substitution; a missing fallback still produces an empty string. A comment notes the regex’s whitespace capture behavior.
  • observed — Modified behavior in apps/web/src/server/utils/contact-variable-replacement.unit.test.ts: Added a test for whitespace-padded fallback values on a nullable built-in contact variable; it expects the replacement to omit the padding.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: trimming whitespace from contact-variable fallback values.
Linked Issues check ✅ Passed PR #457 implements issue #456 by trimming captured fallback whitespace in replaceContactVariables. The regression test covers fallback= there with a null firstName and expects there. The repo…
Out of Scope Changes check ✅ Passed The reported changes are limited to the shared contact-variable replacement helper and its regression test. Both changes directly support issue #456. No unrelated change is identified in the supplied …
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

apps/web/src/server/utils/contact-variable-replacement.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

apps/web/src/server/utils/contact-variable-replacement.unit.test.ts

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐞 - Whitespace inside {{var, fallback=...}} ends up in the rendered text

1 participant