Conversation
…ES-1451) The handleLinkBlur function was blindly prepending https:// to all field values on blur, including email fields. This produced invalid values like https://info@church.com that triggered browser validation errors and blocked users from proceeding. - Make handleLinkBlur link-type-aware via a linkType parameter - Email fields: store bare addresses, strip mailto: prefix, trim whitespace - Sanitize previously corrupted email values on form load - Add placeholder text to email/URL fields for clarity - Fix handleLinkBLur typo (capital B) to handleLinkBlur Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR fixes the "Contact Us" email auto-normalization behavior on the Template Customization Links screen by updating the blur handler to apply link-type-specific normalization: email fields strip Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
View your CI Pipeline Execution ↗ for commit 84c929a
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit ba77e45
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/plans/2026-04-06-001-fix-contact-us-email-https-prepend-plan.md (1)
45-66: Minor doc inconsistency: function name typo was also fixed in implementation.The plan shows the proposed fix still using
handleLinkBLur(capital B typo), but the actual implementation correctly renamed it tohandleLinkBlur. This is a good additional fix that could be reflected in the plan for accuracy.📝 Suggested documentation update
-**1a. Modify `handleLinkBLur` signature and logic (lines 70-75)** +**1a. Modify `handleLinkBlur` signature and logic (lines 70-75)** + +Note: The function was also renamed from `handleLinkBLur` to `handleLinkBlur` to fix the capitalization typo. ```typescript -function handleLinkBLur( +function handleLinkBlur( e: React.FocusEvent<HTMLInputElement>, linkType: 'url' | 'email' | 'chatButtons' ): void {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-04-06-001-fix-contact-us-email-https-prepend-plan.md` around lines 45 - 66, Update the plan to match the implemented function name change: replace any occurrences of the misspelled handleLinkBLur with the corrected handleLinkBlur and ensure all references (including examples and call sites described in the plan) use handleLinkBlur; also verify the documented behavior for 'email' vs 'url'/'chatButtons' in the plan matches the implemented logic in handleLinkBlur (stripping "mailto:" for emails and prepending "https://" for URLs/chatButtons).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/plans/2026-04-06-001-fix-contact-us-email-https-prepend-plan.md`:
- Around line 45-66: Update the plan to match the implemented function name
change: replace any occurrences of the misspelled handleLinkBLur with the
corrected handleLinkBlur and ensure all references (including examples and call
sites described in the plan) use handleLinkBlur; also verify the documented
behavior for 'email' vs 'url'/'chatButtons' in the plan matches the implemented
logic in handleLinkBlur (stripping "mailto:" for emails and prepending
"https://" for URLs/chatButtons).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3ec09284-f215-40d6-924a-15b0a496850a
📒 Files selected for processing (4)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksForm/LinksForm.spec.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksForm/LinksForm.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksScreen.tsxdocs/plans/2026-04-06-001-fix-contact-us-email-https-prepend-plan.md
Contributor
|
The latest updates on your projects.
|
…NES-1451) MUI TextField onBlur provides FocusEvent<HTMLInputElement | HTMLTextAreaElement> but handleLinkBlur only accepted FocusEvent<HTMLInputElement>, causing a TS2345 type error in CI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksForm/LinksForm.tsx`:
- Around line 74-84: The input handler in LinksForm is trimming after doing
normalization which lets values like " mailto:user@example.com " or
whitespace-only strings produce malformed results; update the onChange/onBlur
logic in LinksForm (where it reads e.target, linkType and calls setFieldValue)
to trim the raw value first, return early if trimmed is empty, then for linkType
=== 'email' strip a leading 'mailto:' from the already-trimmed value before
calling setFieldValue, otherwise run the URL normalization (prepend 'https://'
if missing) against the trimmed value and pass that to setFieldValue.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f27314d1-e05b-411b-b8c9-ae66015439ea
📒 Files selected for processing (1)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksForm/LinksForm.tsx
Trimming after normalization allowed edge cases: - " mailto:user@example.com " bypassed the mailto: strip (leading space) - " example.com " produced "https:// example.com " (embedded spaces) - " " (whitespace-only) passed the truthy check and triggered a no-op setFieldValue Now trims first, then early-returns on empty, then normalizes the clean value. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…S-1451) - Add URL placeholder test (was removed earlier due to JSDOM quirk, now using container.querySelector instead of getByPlaceholderText) - Add whitespace-only input test (trim-first early return) - Add spaces-around-mailto test (trim before startsWith check) - Fix URL placeholder: remove t() wrapper since i18next interprets the colon in "https:" as a namespace separator, producing "//example.com" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksForm/LinksForm.tsx`:
- Around line 78-80: The code that strips "mailto:" from the trimmed input only
checks for lowercase 'mailto:' so values like 'MAILTO:user@x' remain
unnormalized; update the logic around linkType/email handling (the branch that
uses trimmed, bare and setFieldValue(name, bare)) to perform a case-insensitive
removal (e.g., test trimmed lowercased or use a case-insensitive regex for
'^mailto:') before slicing so the bare variable always contains the actual email
string when calling setFieldValue.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0921ebb9-13be-454c-9235-ad7b4a4188e8
📒 Files selected for processing (3)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksForm/LinksForm.spec.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksForm/LinksForm.tsxdocs/plans/2026-04-06-001-fix-contact-us-email-https-prepend-plan.md
✅ Files skipped from review due to trivial changes (1)
- apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksForm/LinksForm.spec.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/plans/2026-04-06-001-fix-contact-us-email-https-prepend-plan.md
…n test (NES-1451) - Use case-insensitive regex for mailto: detection so MAILTO:user@example.com is also normalized to a bare email address - Add LinksScreen integration test verifying that corrupted stored email values (https://user@example.com) are sanitized to bare addresses on form load Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sanitization (NES-1451)
- Use .toLowerCase().startsWith('mailto:') instead of regex — simpler
and more readable
- Remove initialValues sanitization for corrupted email values — the
editor validates with .email() so https:// can never be stored, and
the bug itself blocked form submission so no corrupt data was persisted
- Remove the corresponding LinksScreen integration test
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Mark status as complete, add PR link - Update handleLinkBlur code to show trim-first, case-insensitive mailto, widened event type, and renamed function - Remove File 2 (LinksScreen sanitization) with explanation of why it was unnecessary - Update edge cases table with whitespace and case-insensitive rows - Remove placeholder t() wrapping note (i18next colon issue) - Check off all acceptance criteria Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…l boolean (NES-1451) The only distinction that matters is email vs not-email. chatButtons and url both get the same https:// prepend treatment, so a three-value union type was unnecessary complexity. A boolean is clearer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…S-1451) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
csiyang
approved these changes
Apr 6, 2026
…ontact-us-link-for-journey-created
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksForm/LinksForm.tsx (1)
290-294: Uset(...)for placeholder strings for i18n consistency.These placeholders are user-facing strings in a TSX file and should follow the project translation pattern.
As per coding guidelines, "Use hook useTranslation('apps-journeys-admin') for accessing translations in TypeScript/React component files" and "Use inline English strings as keys in translations (e.g., t('Edit {{title}}', { title }))".Suggested patch
- placeholder={ - link.linkType === 'email' - ? 'email@example.com' - : 'https://example.com' - } + placeholder={ + link.linkType === 'email' + ? t('email@example.com') + : t('https://example.com') + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksForm/LinksForm.tsx` around lines 290 - 294, The placeholder strings in LinksForm.tsx are hardcoded and must use the translation hook; import and call useTranslation('apps-journeys-admin') in the LinksForm component (e.g., const { t } = useTranslation('apps-journeys-admin')) and replace the conditional placeholders (the branch using link.linkType === 'email') with translation lookups (e.g., t('email@example.com') and t('https://example.com')) following the project's inline-English-key convention so the input placeholder uses t(...) instead of raw literals.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksForm/LinksForm.tsx`:
- Around line 86-87: The current normalization uses /^\w+:\/\// and blindly
prepends "https://" which mangles protocol-relative URLs like "//example.com";
update the logic in LinksForm where url is derived (the regex /^\w+:\/\// and
setFieldValue usage) to mirror handleOpenLink’s protocol handling: first check
if trimmed startsWith("//") and prefix "https:" (not "https://"), else if
/^\w+:\/\// matches use trimmed as-is, otherwise prefix "https://", then call
setFieldValue(name, url).
---
Nitpick comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksForm/LinksForm.tsx`:
- Around line 290-294: The placeholder strings in LinksForm.tsx are hardcoded
and must use the translation hook; import and call
useTranslation('apps-journeys-admin') in the LinksForm component (e.g., const {
t } = useTranslation('apps-journeys-admin')) and replace the conditional
placeholders (the branch using link.linkType === 'email') with translation
lookups (e.g., t('email@example.com') and t('https://example.com')) following
the project's inline-English-key convention so the input placeholder uses t(...)
instead of raw literals.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 41d47272-fb92-4a92-bcba-9383c492b85f
📒 Files selected for processing (2)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksForm/LinksForm.tsxdocs/plans/2026-04-06-001-fix-contact-us-email-https-prepend-plan.md
✅ Files skipped from review due to trivial changes (1)
- docs/plans/2026-04-06-001-fix-contact-us-email-https-prepend-plan.md
Comment on lines
+86
to
87
| const url = /^\w+:\/\//.test(trimmed) ? trimmed : `https://${trimmed}` | ||
| void setFieldValue(name, url) |
Contributor
There was a problem hiding this comment.
Protocol-relative URLs can be malformed during normalization.
//example.com fails ^\w+:\/\/ and becomes https:////example.com. Consider aligning this check with handleOpenLink’s protocol logic.
Suggested patch
- const url = /^\w+:\/\//.test(trimmed) ? trimmed : `https://${trimmed}`
+ const hasProtocol = /^(https?:)?\/\//i.test(trimmed)
+ const url = hasProtocol ? trimmed : `https://${trimmed}`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksForm/LinksForm.tsx`
around lines 86 - 87, The current normalization uses /^\w+:\/\// and blindly
prepends "https://" which mangles protocol-relative URLs like "//example.com";
update the logic in LinksForm where url is derived (the regex /^\w+:\/\// and
setFieldValue usage) to mirror handleOpenLink’s protocol handling: first check
if trimmed startsWith("//") and prefix "https:" (not "https://"), else if
/^\w+:\/\// matches use trimmed as-is, otherwise prefix "https://", then call
setFieldValue(name, url).
jianwei1
deleted the
jianweichong/nes-1451-cannot-edit-email-in-contact-us-link-for-journey-created
branch
tanflem
pushed a commit
that referenced
this pull request
Apr 13, 2026
…#8950) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
handleLinkBlurfunction on the template customization Links screen was blindly prependinghttps://to all field values on blur, including email fields. This produced invalid values likehttps://info@yourchurch.comthat triggered browser validation errors and blocked users from proceeding.handleLinkBlurlink-type-aware so email fields store bare addresses (no protocol prefix). Also stripsmailto:(case-insensitive) if pasted, trims all values before normalization, and adds placeholder text (email@example.com/https://example.com) to clarify expected input format.handleLinkBLurtypo (capital B) tohandleLinkBlur. Widened event type toHTMLInputElement | HTMLTextAreaElementto match MUI'sonBlursignature.Changed Files
LinksForm.tsxhandleLinkBLur→handleLinkBlur, addlinkTypeparam, trim-first logic, case-insensitivemailto:strip, widen event type, updateonBlurbindings, add placeholders (not wrapped int()— colon breaks i18next)LinksForm.spec.tsxTest plan
info@yourchurch.com) in the Contact Us field → blur → verify nohttps://prependmailto:info@yourchurch.com→ blur → verify it normalizes toinfo@yourchurch.comMAILTO:info@yourchurch.com→ blur → verify case-insensitive strip worksemail@example.complaceholder when emptyhttps://prepended on blur (no regression)https://example.complaceholder when emptyhttps://prepended on blur🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
https://prefixes and properly stripmailto:prefixes when present.email@example.comfor email,https://example.comfor URLs).Tests