feat: email onboarding wizard + signature setup#183
Merged
Systemsaholic merged 16 commits intomainfrom Apr 12, 2026
Merged
Conversation
Two-step wizard during onboarding (connect email + signature), plus profile email tab improvements for existing users. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Key revisions: - Signature saves as signatureHtml (what senders use), not structured-only - Test-connection before create (two-step API flow) - Set onboardingCompletedAt on wizard completion - Hardcoded Phoenix Voyages constants for V1 - Move signature editing from Preferences to Email tab Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- New user_profiles columns: designations, job_title, phone_extension - Agency business config exposed via API for signature builder - All signature fields sourced from real data, not hardcoded Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
10 tasks: migration, types, backend, agent info tab, signature builder, wizard component, profile page wiring, email tab improvements, trip-order business config, testing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… business config Adds agent identity fields (designations, job_title, phone_extension) to user_profiles and business detail fields (company_phone, company_toll_free, company_email, company_address, tico_registration) to agency_settings with Phoenix Voyages seed data. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add designations, jobTitle, phoneExtension to UpdateUserProfileDto (validation decorators) - Wire new fields into updateMyProfile() updateData builder - Add getAgencyBusinessConfig() querying agency_settings + agencies tables - Update getMyProfile() to call getAgencyBusinessConfig() and include result in response - Add AgencyBusinessConfigDto interface + agencyBusinessConfig field to UserProfileResponseDto in shared-types Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pure function that constructs an HTML email signature from structured agent/company data, supporting avatar, TICO registration, tagline, microsite URL, and phone extension fields. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two-step card-based wizard for new user onboarding: Step 1 tests IMAP connection and creates the email account, Step 2 configures the email signature with live preview, tagline, and avatar toggle. Extends EmailSignatureConfigDto with tagline and showAvatar fields. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds ProfilePageRouter inside Suspense to detect ?setup=true and render EmailSetupWizard instead of the normal profile tabs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…f hardcoding Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ement here - Email tab: pre-fills emailAddress/username/server fields from profile for @phoenixvoyages.ca users; server fields disabled when domain matches - Email tab: adds live signature preview, avatar toggle, tagline textarea, and Update Signature button that generates HTML via buildSignatureHtml and saves to emailSignatureConfig - Preferences tab: removes signature form fields (enabled, html, includeInReplies) from interface/form/submit; replaces signature Card content with redirect notice pointing to Email tab Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds an email onboarding wizard and client-side signature builder; extends user profiles with designations/jobTitle/phoneExtension; adds agency business contact fields; updates API to persist and return these fields; introduces DB migrations and shared types; moves signature UI into the Email tab and exposes the wizard at /profile?setup=true. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Client as Email Setup<br/>Wizard (Client)
participant API as API Server
participant DB as Database
participant Profile as UserProfile<br/>Service
User->>Client: Open /profile?setup=true
Client->>Profile: GET /user-profiles/me
Profile->>API: fetch agencyBusinessConfig
API->>DB: read user_profiles, agency_settings
DB-->>API: profile + agency settings
API-->>Client: profile + agencyBusinessConfig
rect rgba(100, 150, 200, 0.5)
note over User,Client: Step 1 — Connect Email
User->>Client: Submit email password
Client->>API: POST /email-accounts/test-connection (IMAP phoenixvoyages)
API->>API: test IMAP
API-->>Client: success
Client->>API: POST /email-accounts (create)
API->>DB: insert email account
DB-->>API: created
API-->>Client: account created
end
rect rgba(150, 100, 200, 0.5)
note over User,Client: Step 2 — Configure Signature
User->>Client: Edit tagline / toggle avatar
Client->>Client: buildSignatureHtml(profile + tagline + avatar)
Client->>Client: render live preview
User->>Client: Save signature
Client->>API: PUT /user-profiles/me (emailSignatureConfig, onboardingCompletedAt)
API->>DB: update user_profiles
DB-->>API: updated
API-->>Client: success
end
rect rgba(200, 150, 100, 0.5)
note over User,Client: Step 3 — Success
Client->>User: show confirmation, navigate to /profile
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
apps/admin/src/app/profile/_components/agent-info-tab.tsx (1)
192-220: Make the Agent Identity grid responsive on small screens.Using a fixed 3-column layout here can compress inputs too much on narrow viewports.
💡 Suggested tweak
- <div className="grid grid-cols-3 gap-4"> + <div className="grid grid-cols-1 gap-4 md:grid-cols-3">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/app/profile/_components/agent-info-tab.tsx` around lines 192 - 220, The Agent Identity grid currently uses a fixed three-column class ("grid grid-cols-3 gap-4") which squashes inputs on small viewports; update the container to use responsive Tailwind classes (e.g., "grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 gap-4") so the inputs for Job Title, Designations, and Phone Extension (registered via form.register('jobTitle'), form.register('designations'), form.register('phoneExtension')) stack on small screens and expand on larger screens; ensure no other layout or spacing classes on the child divs are removed so Labels and Inputs keep their spacing.apps/admin/src/app/profile/_components/email-setup-wizard.tsx (1)
105-110: **Consider adding Sentry error capture for runtime errors.**Per the coding guidelines, the admin application should capture runtime errors with Sentry. Errors caught in try/catch blocks should useSentry.captureException(err)to report them. The error handlers in this wizard catch exceptions but only display them locally without reporting to Sentry.📊 Proposed Sentry integration
+'use client' + +import * as Sentry from '@sentry/react' import { useState, useMemo } from 'react' // ... other imports async function handleTestAndConnect() { // ... } catch (err: unknown) { const message = err instanceof Error ? err.message : 'An unexpected error occurred.' + if (err instanceof Error) { + Sentry.captureException(err, { tags: { feature: 'email-setup-wizard', step: 'connect' } }) + } setError(message) } finally { setIsConnecting(false) } } async function handleSaveSignature() { // ... } catch (err: unknown) { const message = err instanceof Error ? err.message : 'Failed to save signature.' + if (err instanceof Error) { + Sentry.captureException(err, { tags: { feature: 'email-setup-wizard', step: 'signature' } }) + } setError(message) } finally { setIsSaving(false) } }As per coding guidelines: "Admin application must capture runtime errors with Sentry."
Also applies to: 152-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/app/profile/_components/email-setup-wizard.tsx` around lines 105 - 110, The catch blocks in the EmailSetupWizard component (where setError and setIsConnecting are used) swallow exceptions without reporting; update each catch handler (including the one around lines using setError/setIsConnecting and the similar handler at the 152–157 region) to call Sentry.captureException(err) before or after computing the user-facing message so runtime errors are sent to Sentry; ensure Sentry is imported/initialized in this module or use the existing Sentry instance and still call setError/setIsConnecting as currently implemented.
🤖 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/admin/src/app/profile/_components/email-tab.tsx`:
- Around line 438-444: The signature preview uses dangerouslySetInnerHTML with
user-controlled fields; update buildSignatureHtml to HTML-escape (or sanitize)
the tagline and any other user inputs before concatenating into the signature
HTML so signaturePreviewHtml never contains raw user markup; locate
buildSignatureHtml and replace direct insertion of tagline with an
escaped/sanitized version (or apply a trusted sanitizer library) and ensure
callers like the component rendering signaturePreviewHtml receive the safe HTML
string.
- Around line 64-70: The useEffect that pre-fills the form reads profile?.email
and calls form.setValue but does not include form in its dependency array;
update the dependency array to include the form (or better, destructure the
stable setter: const { setValue } = form and use setValue in the effect while
adding setValue to the deps) so that when the form instance changes the effect
re-runs and calls form.setValue('emailAddress', profile.email) and
form.setValue('username', profile.email) with the correct form reference.
- Around line 137-167: The handleUpdateSignature function currently swallows
errors in its try/finally; modify it to catch errors from
updateProfile.mutateAsync and display user-facing feedback (e.g., a toast or
notification) before rethrowing or returning, while still ensuring
setIsSavingSignature(false) runs in the finally block; specifically add a
catch(err) that calls the existing toast/notification helper with a clear
message and the error details, referencing handleUpdateSignature and
updateProfile.mutateAsync so the UI shows an error when signature update fails.
In `@apps/admin/src/lib/email/build-signature-html.ts`:
- Around line 17-73: The buildSignatureHtml function directly interpolates
user-controlled fields (firstName, lastName, designations, jobTitle, tagline,
companyName, companyPhone, companyAddress, ticoRegistration, microSiteUrl,
avatarUrl) into HTML, creating XSS and javascript:-URL injection risks; fix by
introducing and using an HTML-escaping helper (e.g., escapeHtml) for all text
content before concatenation and by validating/sanitizing URLs (microSiteUrl,
avatarUrl) to allow only http(s) origins and percent-encode/normalize them
before placing in href/src attributes (if invalid, omit the link/image). Ensure
you call escapeHtml for values used in element text or attribute values inside
buildSignatureHtml and perform URL validation for microSiteUrl and avatarUrl
prior to including them.
In `@apps/api/src/user-profiles/dto/update-user-profile.dto.ts`:
- Around line 218-228: The DTO properties designations, jobTitle, and
phoneExtension on the UpdateUserProfileDto are missing length bounds; add
appropriate `@MaxLength`() validators (and `@IsOptional`()/@IsString() already
present) to cap their sizes (e.g. designations ~ 256, jobTitle ~ 100,
phoneExtension ~ 10 or values matching DB schema), import MaxLength from
class-validator, and update the UpdateUserProfileDto property decorators for
designations, jobTitle, and phoneExtension accordingly so oversized values fail
validation before persistence.
In
`@packages/database/src/migrations/20260411120000_add_profile_and_agency_business_fields.sql`:
- Around line 14-21: The UPDATE only changes existing rows in agency_settings so
the Phoenix agency may never be seeded; change the step to an idempotent upsert:
perform an INSERT into agency_settings specifying agency_id =
'00000000-0000-0000-0000-000000000001' and all columns (company_phone,
company_toll_free, company_email, company_address, tico_registration,
updated_at) with updated_at = NOW(), and use ON CONFLICT (agency_id) DO UPDATE
SET to update those same columns so the row is created if missing and updated if
present (refer to the agency_settings table and the agency_id value in the
diff).
---
Nitpick comments:
In `@apps/admin/src/app/profile/_components/agent-info-tab.tsx`:
- Around line 192-220: The Agent Identity grid currently uses a fixed
three-column class ("grid grid-cols-3 gap-4") which squashes inputs on small
viewports; update the container to use responsive Tailwind classes (e.g., "grid
grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 gap-4") so the inputs for Job Title,
Designations, and Phone Extension (registered via form.register('jobTitle'),
form.register('designations'), form.register('phoneExtension')) stack on small
screens and expand on larger screens; ensure no other layout or spacing classes
on the child divs are removed so Labels and Inputs keep their spacing.
In `@apps/admin/src/app/profile/_components/email-setup-wizard.tsx`:
- Around line 105-110: The catch blocks in the EmailSetupWizard component (where
setError and setIsConnecting are used) swallow exceptions without reporting;
update each catch handler (including the one around lines using
setError/setIsConnecting and the similar handler at the 152–157 region) to call
Sentry.captureException(err) before or after computing the user-facing message
so runtime errors are sent to Sentry; ensure Sentry is imported/initialized in
this module or use the existing Sentry instance and still call
setError/setIsConnecting as currently implemented.
🪄 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
Run ID: 3eaf2d0c-dcf1-463b-8851-cff8e1b786ba
📒 Files selected for processing (16)
apps/admin/src/app/profile/_components/agent-info-tab.tsxapps/admin/src/app/profile/_components/email-setup-wizard.tsxapps/admin/src/app/profile/_components/email-tab.tsxapps/admin/src/app/profile/_components/preferences-tab.tsxapps/admin/src/app/profile/page.tsxapps/admin/src/lib/email/build-signature-html.tsapps/api/src/financials/trip-order.service.tsapps/api/src/user-profiles/dto/update-user-profile.dto.tsapps/api/src/user-profiles/user-profiles.service.tsdocs/superpowers/plans/2026-04-11-email-onboarding-wizard.mddocs/superpowers/specs/2026-04-11-email-onboarding-wizard-design.mdpackages/database/src/migrations/20260411120000_add_profile_and_agency_business_fields.sqlpackages/database/src/migrations/meta/_journal.jsonpackages/database/src/schema/financials.schema.tspackages/database/src/schema/user-profiles.schema.tspackages/shared-types/src/api/user-profiles.types.ts
Comment on lines
+64
to
+70
| // Pre-fill from profile when it loads | ||
| useEffect(() => { | ||
| if (profile?.email) { | ||
| form.setValue('emailAddress', profile.email) | ||
| form.setValue('username', profile.email) | ||
| } | ||
| }, [profile?.email]) |
There was a problem hiding this comment.
Missing form in useEffect dependency array.
The effect calls form.setValue but form is not included in the dependency array. This could cause stale closure issues if the form instance changes.
🔧 Proposed fix
useEffect(() => {
if (profile?.email) {
form.setValue('emailAddress', profile.email)
form.setValue('username', profile.email)
}
- }, [profile?.email])
+ }, [profile?.email, form])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Pre-fill from profile when it loads | |
| useEffect(() => { | |
| if (profile?.email) { | |
| form.setValue('emailAddress', profile.email) | |
| form.setValue('username', profile.email) | |
| } | |
| }, [profile?.email]) | |
| // Pre-fill from profile when it loads | |
| useEffect(() => { | |
| if (profile?.email) { | |
| form.setValue('emailAddress', profile.email) | |
| form.setValue('username', profile.email) | |
| } | |
| }, [profile?.email, form]) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/app/profile/_components/email-tab.tsx` around lines 64 - 70,
The useEffect that pre-fills the form reads profile?.email and calls
form.setValue but does not include form in its dependency array; update the
dependency array to include the form (or better, destructure the stable setter:
const { setValue } = form and use setValue in the effect while adding setValue
to the deps) so that when the form instance changes the effect re-runs and calls
form.setValue('emailAddress', profile.email) and form.setValue('username',
profile.email) with the correct form reference.
Comment on lines
+137
to
+167
| const handleUpdateSignature = async () => { | ||
| if (!profile) return | ||
| setIsSavingSignature(true) | ||
| try { | ||
| const signatureHtml = buildSignatureHtml({ | ||
| firstName: profile.firstName || '', | ||
| lastName: profile.lastName || '', | ||
| designations: profile.designations, | ||
| jobTitle: profile.jobTitle, | ||
| phoneExtension: profile.phoneExtension, | ||
| avatarUrl: profile.avatarUrl, | ||
| microSiteUrl: undefined, | ||
| companyName: profile.agencyBusinessConfig?.agencyName || '', | ||
| companyPhone: profile.agencyBusinessConfig?.companyPhone, | ||
| companyAddress: profile.agencyBusinessConfig?.companyAddress, | ||
| ticoRegistration: profile.agencyBusinessConfig?.ticoRegistration, | ||
| tagline: tagline || undefined, | ||
| showAvatar, | ||
| }) | ||
| await updateProfile.mutateAsync({ | ||
| emailSignatureConfig: { | ||
| ...profile.emailSignatureConfig, | ||
| tagline: tagline || undefined, | ||
| showAvatar, | ||
| signatureHtml, | ||
| }, | ||
| }) | ||
| } finally { | ||
| setIsSavingSignature(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing error feedback to user on signature update failure.
The handleUpdateSignature function catches errors but doesn't display them to the user via toast. Consider adding error feedback for consistency with other handlers in this codebase.
🔧 Proposed fix
try {
// ... existing code ...
await updateProfile.mutateAsync({
// ... payload ...
})
+ toast({
+ title: 'Signature updated',
+ description: 'Your email signature has been saved.',
+ })
- } finally {
+ } catch (err) {
+ toast({
+ title: 'Error',
+ description: 'Failed to update signature. Please try again.',
+ variant: 'destructive',
+ })
+ } finally {
setIsSavingSignature(false)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/app/profile/_components/email-tab.tsx` around lines 137 - 167,
The handleUpdateSignature function currently swallows errors in its try/finally;
modify it to catch errors from updateProfile.mutateAsync and display user-facing
feedback (e.g., a toast or notification) before rethrowing or returning, while
still ensuring setIsSavingSignature(false) runs in the finally block;
specifically add a catch(err) that calls the existing toast/notification helper
with a clear message and the error details, referencing handleUpdateSignature
and updateProfile.mutateAsync so the UI shows an error when signature update
fails.
Comment on lines
+438
to
+444
| <div className="space-y-2"> | ||
| <Label>Preview</Label> | ||
| <div | ||
| className="rounded-md border bg-white p-4 text-sm" | ||
| dangerouslySetInnerHTML={{ __html: signaturePreviewHtml }} | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
XSS consideration with dangerouslySetInnerHTML and user-controlled tagline.
The static analysis flagged this usage. While most data comes from trusted profile sources, the tagline is user-controlled input that gets embedded in the HTML. The buildSignatureHtml function should escape the tagline to prevent XSS.
Review buildSignatureHtml to ensure user-controlled fields (especially tagline) are HTML-escaped before being inserted into the signature HTML string.
🧰 Tools
🪛 ast-grep (0.42.1)
[warning] 441-441: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/app/profile/_components/email-tab.tsx` around lines 438 - 444,
The signature preview uses dangerouslySetInnerHTML with user-controlled fields;
update buildSignatureHtml to HTML-escape (or sanitize) the tagline and any other
user inputs before concatenating into the signature HTML so signaturePreviewHtml
never contains raw user markup; locate buildSignatureHtml and replace direct
insertion of tagline with an escaped/sanitized version (or apply a trusted
sanitizer library) and ensure callers like the component rendering
signaturePreviewHtml receive the safe HTML string.
Comment on lines
+17
to
+73
| export function buildSignatureHtml(data: SignatureData): string { | ||
| const parts: string[] = [] | ||
|
|
||
| // Line 1: Name, Designations | Title | ||
| let nameLine = `<strong>${data.firstName} ${data.lastName}</strong>` | ||
| if (data.designations) nameLine += `, ${data.designations}` | ||
| if (data.jobTitle) nameLine += ` | ${data.jobTitle}` | ||
| parts.push(nameLine) | ||
|
|
||
| // Tagline (if set) | ||
| if (data.tagline) { | ||
| parts.push(`<em style="color:#6b7280;">${data.tagline}</em>`) | ||
| } | ||
|
|
||
| // MicroSite URL | ||
| if (data.microSiteUrl) { | ||
| parts.push(`<a href="${data.microSiteUrl}" style="color:#c59746;">${data.microSiteUrl}</a>`) | ||
| } | ||
|
|
||
| // Company name | ||
| parts.push(data.companyName) | ||
|
|
||
| // Phone with extension | ||
| if (data.companyPhone) { | ||
| let phoneLine = data.companyPhone | ||
| if (data.phoneExtension) phoneLine += ` ext ${data.phoneExtension}` | ||
| parts.push(phoneLine) | ||
| } | ||
|
|
||
| // Address | ||
| if (data.companyAddress) { | ||
| parts.push(data.companyAddress) | ||
| } | ||
|
|
||
| // TICO | ||
| if (data.ticoRegistration) { | ||
| parts.push(`<span style="font-size:11px;color:#6b7280;">TICO Ontario Registration No: ${data.ticoRegistration}</span>`) | ||
| } | ||
|
|
||
| // Build the HTML | ||
| let html = '<div style="font-family:Arial,Helvetica,sans-serif;font-size:13px;color:#333;line-height:1.5;">' | ||
|
|
||
| // Avatar (optional, floated left) | ||
| if (data.showAvatar && data.avatarUrl) { | ||
| html += `<img src="${data.avatarUrl}" alt="${data.firstName} ${data.lastName}" style="width:60px;height:60px;border-radius:50%;float:left;margin-right:12px;margin-bottom:8px;" />` | ||
| } | ||
|
|
||
| html += parts.join('<br />') | ||
| html += '</div>' | ||
|
|
||
| // Clear float | ||
| if (data.showAvatar && data.avatarUrl) { | ||
| html += '<div style="clear:both;"></div>' | ||
| } | ||
|
|
||
| return html | ||
| } |
There was a problem hiding this comment.
Critical: No HTML escaping on user-controlled inputs — XSS vulnerability.
This function constructs HTML by directly interpolating user-controlled values (firstName, lastName, designations, jobTitle, tagline, etc.) without escaping. When rendered via dangerouslySetInnerHTML, malicious input like <script>alert(1)</script> in the tagline field will execute.
Additionally, microSiteUrl and avatarUrl should be validated to prevent javascript: URL injection.
🔒 Proposed fix: Add HTML escaping utility
+function escapeHtml(str: string): string {
+ return str
+ .replace(/&/g, '&')
+ .replace(/</g, '<')
+ .replace(/>/g, '>')
+ .replace(/"/g, '"')
+ .replace(/'/g, ''')
+}
+
+function sanitizeUrl(url: string | null | undefined): string | null {
+ if (!url) return null
+ // Only allow http(s) URLs
+ if (url.startsWith('http://') || url.startsWith('https://')) {
+ return url
+ }
+ return null
+}
+
export function buildSignatureHtml(data: SignatureData): string {
const parts: string[] = []
// Line 1: Name, Designations | Title
- let nameLine = `<strong>${data.firstName} ${data.lastName}</strong>`
- if (data.designations) nameLine += `, ${data.designations}`
- if (data.jobTitle) nameLine += ` | ${data.jobTitle}`
+ let nameLine = `<strong>${escapeHtml(data.firstName)} ${escapeHtml(data.lastName)}</strong>`
+ if (data.designations) nameLine += `, ${escapeHtml(data.designations)}`
+ if (data.jobTitle) nameLine += ` | ${escapeHtml(data.jobTitle)}`
parts.push(nameLine)
// Tagline (if set)
if (data.tagline) {
- parts.push(`<em style="color:`#6b7280`;">${data.tagline}</em>`)
+ parts.push(`<em style="color:`#6b7280`;">${escapeHtml(data.tagline)}</em>`)
}
// MicroSite URL
- if (data.microSiteUrl) {
- parts.push(`<a href="${data.microSiteUrl}" style="color:`#c59746`;">${data.microSiteUrl}</a>`)
+ const safeMicroSiteUrl = sanitizeUrl(data.microSiteUrl)
+ if (safeMicroSiteUrl) {
+ parts.push(`<a href="${safeMicroSiteUrl}" style="color:`#c59746`;">${escapeHtml(safeMicroSiteUrl)}</a>`)
}
// ... apply escapeHtml to companyName, companyPhone, companyAddress, ticoRegistration similarly🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/lib/email/build-signature-html.ts` around lines 17 - 73, The
buildSignatureHtml function directly interpolates user-controlled fields
(firstName, lastName, designations, jobTitle, tagline, companyName,
companyPhone, companyAddress, ticoRegistration, microSiteUrl, avatarUrl) into
HTML, creating XSS and javascript:-URL injection risks; fix by introducing and
using an HTML-escaping helper (e.g., escapeHtml) for all text content before
concatenation and by validating/sanitizing URLs (microSiteUrl, avatarUrl) to
allow only http(s) origins and percent-encode/normalize them before placing in
href/src attributes (if invalid, omit the link/image). Ensure you call
escapeHtml for values used in element text or attribute values inside
buildSignatureHtml and perform URL validation for microSiteUrl and avatarUrl
prior to including them.
Comment on lines
+218
to
+228
| @IsOptional() | ||
| @IsString() | ||
| designations?: string | ||
|
|
||
| @IsOptional() | ||
| @IsString() | ||
| jobTitle?: string | ||
|
|
||
| @IsOptional() | ||
| @IsString() | ||
| phoneExtension?: string |
There was a problem hiding this comment.
Add length validation to new profile fields.
These fields are string-validated but not length-bounded, so oversized values can pass DTO validation and fail later at persistence.
💡 Suggested fix
`@IsOptional`()
`@IsString`()
+ `@MaxLength`(255)
designations?: string
`@IsOptional`()
`@IsString`()
+ `@MaxLength`(100)
jobTitle?: string
`@IsOptional`()
`@IsString`()
+ `@MaxLength`(20)
phoneExtension?: string📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @IsOptional() | |
| @IsString() | |
| designations?: string | |
| @IsOptional() | |
| @IsString() | |
| jobTitle?: string | |
| @IsOptional() | |
| @IsString() | |
| phoneExtension?: string | |
| `@IsOptional`() | |
| `@IsString`() | |
| `@MaxLength`(255) | |
| designations?: string | |
| `@IsOptional`() | |
| `@IsString`() | |
| `@MaxLength`(100) | |
| jobTitle?: string | |
| `@IsOptional`() | |
| `@IsString`() | |
| `@MaxLength`(20) | |
| phoneExtension?: string |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/user-profiles/dto/update-user-profile.dto.ts` around lines 218 -
228, The DTO properties designations, jobTitle, and phoneExtension on the
UpdateUserProfileDto are missing length bounds; add appropriate `@MaxLength`()
validators (and `@IsOptional`()/@IsString() already present) to cap their sizes
(e.g. designations ~ 256, jobTitle ~ 100, phoneExtension ~ 10 or values matching
DB schema), import MaxLength from class-validator, and update the
UpdateUserProfileDto property decorators for designations, jobTitle, and
phoneExtension accordingly so oversized values fail validation before
persistence.
Comment on lines
+14
to
+21
| UPDATE agency_settings SET | ||
| company_phone = '(855) 383-5771', | ||
| company_toll_free = '(855) 383-5771', | ||
| company_email = 'info@phoenixvoyages.ca', | ||
| company_address = '600 Du Golf Rd, Hammond ON K0A2A0', | ||
| tico_registration = '50028032', | ||
| updated_at = NOW() | ||
| WHERE agency_id = '00000000-0000-0000-0000-000000000001'; |
There was a problem hiding this comment.
Seed step is not guaranteed to run because it only updates existing rows.
If agency_settings has no row yet for the Phoenix agency, this update does nothing and the intended seeded business config is missing.
💡 Suggested fix (idempotent upsert)
-UPDATE agency_settings SET
- company_phone = '(855) 383-5771',
- company_toll_free = '(855) 383-5771',
- company_email = 'info@phoenixvoyages.ca',
- company_address = '600 Du Golf Rd, Hammond ON K0A2A0',
- tico_registration = '50028032',
- updated_at = NOW()
-WHERE agency_id = '00000000-0000-0000-0000-000000000001';
+INSERT INTO agency_settings (
+ agency_id,
+ company_phone,
+ company_toll_free,
+ company_email,
+ company_address,
+ tico_registration,
+ updated_at
+)
+VALUES (
+ '00000000-0000-0000-0000-000000000001',
+ '(855) 383-5771',
+ '(855) 383-5771',
+ 'info@phoenixvoyages.ca',
+ '600 Du Golf Rd, Hammond ON K0A2A0',
+ '50028032',
+ NOW()
+)
+ON CONFLICT (agency_id) DO UPDATE SET
+ company_phone = EXCLUDED.company_phone,
+ company_toll_free = EXCLUDED.company_toll_free,
+ company_email = EXCLUDED.company_email,
+ company_address = EXCLUDED.company_address,
+ tico_registration = EXCLUDED.tico_registration,
+ updated_at = NOW();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| UPDATE agency_settings SET | |
| company_phone = '(855) 383-5771', | |
| company_toll_free = '(855) 383-5771', | |
| company_email = 'info@phoenixvoyages.ca', | |
| company_address = '600 Du Golf Rd, Hammond ON K0A2A0', | |
| tico_registration = '50028032', | |
| updated_at = NOW() | |
| WHERE agency_id = '00000000-0000-0000-0000-000000000001'; | |
| INSERT INTO agency_settings ( | |
| agency_id, | |
| company_phone, | |
| company_toll_free, | |
| company_email, | |
| company_address, | |
| tico_registration, | |
| updated_at | |
| ) | |
| VALUES ( | |
| '00000000-0000-0000-0000-000000000001', | |
| '(855) 383-5771', | |
| '(855) 383-5771', | |
| 'info@phoenixvoyages.ca', | |
| '600 Du Golf Rd, Hammond ON K0A2A0', | |
| '50028032', | |
| NOW() | |
| ) | |
| ON CONFLICT (agency_id) DO UPDATE SET | |
| company_phone = EXCLUDED.company_phone, | |
| company_toll_free = EXCLUDED.company_toll_free, | |
| company_email = EXCLUDED.company_email, | |
| company_address = EXCLUDED.company_address, | |
| tico_registration = EXCLUDED.tico_registration, | |
| updated_at = NOW(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/database/src/migrations/20260411120000_add_profile_and_agency_business_fields.sql`
around lines 14 - 21, The UPDATE only changes existing rows in agency_settings
so the Phoenix agency may never be seeded; change the step to an idempotent
upsert: perform an INSERT into agency_settings specifying agency_id =
'00000000-0000-0000-0000-000000000001' and all columns (company_phone,
company_toll_free, company_email, company_address, tico_registration,
updated_at) with updated_at = NOW(), and use ON CONFLICT (agency_id) DO UPDATE
SET to update those same columns so the row is created if missing and updated if
present (refer to the agency_settings table and the agency_id value in the
diff).
…ate gracefully - Check useEmailAccounts on mount — if account exists, skip to signature - Catch duplicate key error on create and proceed to signature instead of showing error Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/admin/src/app/profile/_components/email-setup-wizard.tsx`:
- Around line 44-45: The auto-skip-to-signature flow is overwriting saved
signature prefs by unconditionally setting tagline='' and showAvatar=true (and
includeInReplies=true) when moving to Step 2; update the logic that performs the
skip (the code that calls setTagline, setShowAvatar, and setIncludeInReplies
during the skip) to first read any existing signature config and only initialize
state when no saved value exists (e.g., if current
tagline/showAvatar/includeInReplies are undefined/null), or explicitly use the
existing account's signature values as the defaults; in other words, change the
skip-to-signature handler to preserve previously saved signature values rather
than overwriting them with the hardcoded defaults.
- Around line 117-118: The duplicate-account detection in the EmailSetupWizard
is brittle because it relies on free-form message text; update the error
handling in the function that processes the backend response (the
submit/submitEmail handler where you inspect message and call setStep) to prefer
structured signals such as an error.code (e.g., "DUPLICATE_ACCOUNT"), an
explicit error.type, or an HTTP 409/409-like status, and only fall back to
substring checks if those structured fields are absent; when detecting the
structured duplicate signal, call setStep('signature') as before. Ensure you
reference and read the backend error object (e.g., err.code / response.status)
rather than only message to make the check robust.
- Around line 114-122: The catch blocks in handleConnectEmail and
handleSaveSignature currently only set UI state; update both to report the
caught error to Sentry before setting UI state by calling
Sentry.captureException(err) (or Sentry.captureMessage when err is not an Error)
and attach the environment tag (e.g., Sentry.setTag('env', process.env.NODE_ENV
|| 'unknown') or Sentry.setContext('env', { app: 'admin', env:
process.env.NODE_ENV })) so errors are recorded with the admin environment; keep
the existing logic (duplicate checks, setStep, setError) after reporting so UX
is unchanged.
- Around line 331-333: The signature HTML is injected via
dangerouslySetInnerHTML using signaturePreviewHtml produced by
buildSignatureHtml(), which currently concatenates user-controlled fields like
tagline without escaping; fix by importing and calling the existing
sanitizeEmailHtml utility to sanitize the HTML returned from buildSignatureHtml
(e.g., const signaturePreviewHtml =
sanitizeEmailHtml(buildSignatureHtml(profile, tagline, ...))) and then use that
sanitized string in the element with dangerouslySetInnerHTML to prevent XSS.
🪄 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
Run ID: ca431be4-3384-435d-b1a3-8fc3d74fe540
📒 Files selected for processing (1)
apps/admin/src/app/profile/_components/email-setup-wizard.tsx
Comment on lines
+44
to
+45
| const [tagline, setTagline] = useState('') | ||
| const [showAvatar, setShowAvatar] = useState(true) |
There was a problem hiding this comment.
Avoid overwriting existing signature preferences when auto-skipping to Step 2.
When an existing email account triggers skip-to-signature, defaults (tagline='', showAvatar=true, includeInReplies=true) can overwrite previously saved signature config on save.
Proposed fix
const [tagline, setTagline] = useState('')
const [showAvatar, setShowAvatar] = useState(true)
...
+ useEffect(() => {
+ const cfg = profile?.emailSignatureConfig
+ if (!cfg) return
+ setTagline(cfg.tagline ?? '')
+ setShowAvatar(cfg.showAvatar ?? true)
+ }, [profile?.emailSignatureConfig])
...
await updateProfile.mutateAsync({
emailSignatureConfig: {
enabled: true,
signatureHtml: html,
- includeInReplies: true,
+ includeInReplies: profile?.emailSignatureConfig?.includeInReplies ?? true,
tagline,
showAvatar,
},Also applies to: 147-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/app/profile/_components/email-setup-wizard.tsx` around lines
44 - 45, The auto-skip-to-signature flow is overwriting saved signature prefs by
unconditionally setting tagline='' and showAvatar=true (and
includeInReplies=true) when moving to Step 2; update the logic that performs the
skip (the code that calls setTagline, setShowAvatar, and setIncludeInReplies
during the skip) to first read any existing signature config and only initialize
state when no saved value exists (e.g., if current
tagline/showAvatar/includeInReplies are undefined/null), or explicitly use the
existing account's signature values as the defaults; in other words, change the
skip-to-signature handler to preserve previously saved signature values rather
than overwriting them with the hardcoded defaults.
Comment on lines
+114
to
+122
| } catch (err: unknown) { | ||
| const message = err instanceof Error ? err.message : 'An unexpected error occurred.' | ||
| // If account already exists (duplicate), just proceed to signature | ||
| if (message.includes('duplicate') || message.includes('already exists') || message.includes('unique constraint')) { | ||
| setStep('signature') | ||
| return | ||
| } | ||
| setError(message) | ||
| } finally { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the specific file
fd -t f 'email-setup-wizard.tsx' apps/admin/Repository: Systemsaholic/tailfire
Length of output: 127
🏁 Script executed:
# If file exists, read the relevant sections with context
cat -n apps/admin/src/app/profile/_components/email-setup-wizard.tsx | sed -n '100,180p'Repository: Systemsaholic/tailfire
Length of output: 3113
🏁 Script executed:
# Check Sentry usage patterns in the admin app
rg -t typescript '@sentry' apps/admin/src/ -B 2 -A 2 | head -50Repository: Systemsaholic/tailfire
Length of output: 100
🏁 Script executed:
# Verify Sentry package is available
fd -t f 'package.json' apps/admin/ -x cat {} | grep -A 20 '"dependencies"' | grep -i sentryRepository: Systemsaholic/tailfire
Length of output: 48
🏁 Script executed:
# Read the imports section of the file
cat -n apps/admin/src/app/profile/_components/email-setup-wizard.tsx | head -30Repository: Systemsaholic/tailfire
Length of output: 1259
🏁 Script executed:
# Search for Sentry usage in admin app with correct syntax
rg '@sentry' apps/admin/src/ -A 2 | head -60Repository: Systemsaholic/tailfire
Length of output: 431
🏁 Script executed:
# Check admin app package.json for dependencies
cat apps/admin/package.json | jq '.dependencies | keys[] | select(contains("sentry"))'Repository: Systemsaholic/tailfire
Length of output: 82
🏁 Script executed:
# Also check root package.json and packages directory
cat package.json | jq '.dependencies | keys[] | select(contains("sentry"))'Repository: Systemsaholic/tailfire
Length of output: 48
Capture caught errors with Sentry in both error handlers.
The handleConnectEmail (lines 114-122) and handleSaveSignature (lines 166-168) catch blocks handle errors silently via UI state only. Both must report to Sentry with environment tagging per the admin application guidelines.
Proposed fix
+import * as Sentry from '@sentry/nextjs'
...
} catch (err: unknown) {
+ Sentry.captureException(err, {
+ tags: {
+ environment: process.env.NEXT_PUBLIC_SENTRY_ENVIRONMENT ?? 'development',
+ feature: 'email-setup-wizard',
+ step: 'connect',
+ },
+ })
const message = err instanceof Error ? err.message : 'An unexpected error occurred.'
...
} catch (err: unknown) {
+ Sentry.captureException(err, {
+ tags: {
+ environment: process.env.NEXT_PUBLIC_SENTRY_ENVIRONMENT ?? 'development',
+ feature: 'email-setup-wizard',
+ step: 'signature',
+ },
+ })
const message = err instanceof Error ? err.message : 'Failed to save signature.'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch (err: unknown) { | |
| const message = err instanceof Error ? err.message : 'An unexpected error occurred.' | |
| // If account already exists (duplicate), just proceed to signature | |
| if (message.includes('duplicate') || message.includes('already exists') || message.includes('unique constraint')) { | |
| setStep('signature') | |
| return | |
| } | |
| setError(message) | |
| } finally { | |
| } catch (err: unknown) { | |
| Sentry.captureException(err, { | |
| tags: { | |
| environment: process.env.NEXT_PUBLIC_SENTRY_ENVIRONMENT ?? 'development', | |
| feature: 'email-setup-wizard', | |
| step: 'connect', | |
| }, | |
| }) | |
| const message = err instanceof Error ? err.message : 'An unexpected error occurred.' | |
| // If account already exists (duplicate), just proceed to signature | |
| if (message.includes('duplicate') || message.includes('already exists') || message.includes('unique constraint')) { | |
| setStep('signature') | |
| return | |
| } | |
| setError(message) | |
| } finally { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/app/profile/_components/email-setup-wizard.tsx` around lines
114 - 122, The catch blocks in handleConnectEmail and handleSaveSignature
currently only set UI state; update both to report the caught error to Sentry
before setting UI state by calling Sentry.captureException(err) (or
Sentry.captureMessage when err is not an Error) and attach the environment tag
(e.g., Sentry.setTag('env', process.env.NODE_ENV || 'unknown') or
Sentry.setContext('env', { app: 'admin', env: process.env.NODE_ENV })) so errors
are recorded with the admin environment; keep the existing logic (duplicate
checks, setStep, setError) after reporting so UX is unchanged.
Comment on lines
+117
to
+118
| if (message.includes('duplicate') || message.includes('already exists') || message.includes('unique constraint')) { | ||
| setStep('signature') |
There was a problem hiding this comment.
Use structured error signals for duplicate-account handling.
Matching free-form error text (duplicate, already exists, unique constraint) is brittle and can break onboarding if backend wording changes.
Proposed fix
} catch (err: unknown) {
const message = err instanceof Error ? err.message : 'An unexpected error occurred.'
- // If account already exists (duplicate), just proceed to signature
- if (message.includes('duplicate') || message.includes('already exists') || message.includes('unique constraint')) {
+ const status =
+ typeof err === 'object' &&
+ err !== null &&
+ 'response' in err &&
+ typeof (err as { response?: { status?: number } }).response?.status === 'number'
+ ? (err as { response?: { status?: number } }).response?.status
+ : undefined
+
+ // If account already exists (duplicate), just proceed to signature
+ if (
+ status === 409 ||
+ message.toLowerCase().includes('duplicate') ||
+ message.toLowerCase().includes('already exists') ||
+ message.toLowerCase().includes('unique constraint')
+ ) {
setStep('signature')
return
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/app/profile/_components/email-setup-wizard.tsx` around lines
117 - 118, The duplicate-account detection in the EmailSetupWizard is brittle
because it relies on free-form message text; update the error handling in the
function that processes the backend response (the submit/submitEmail handler
where you inspect message and call setStep) to prefer structured signals such as
an error.code (e.g., "DUPLICATE_ACCOUNT"), an explicit error.type, or an HTTP
409/409-like status, and only fall back to substring checks if those structured
fields are absent; when detecting the structured duplicate signal, call
setStep('signature') as before. Ensure you reference and read the backend error
object (e.g., err.code / response.status) rather than only message to make the
check robust.
Comment on lines
+331
to
+333
| className="rounded-md border bg-white p-4" | ||
| dangerouslySetInnerHTML={{ __html: signaturePreviewHtml }} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/admin/src/app/profile/_components/email-setup-wizard.tsx | head -400Repository: Systemsaholic/tailfire
Length of output: 15002
🏁 Script executed:
fd -type f "build-signature-html" -o -name "*signature*" | grep -E "\.(ts|tsx|js)$"Repository: Systemsaholic/tailfire
Length of output: 236
🏁 Script executed:
fd "build-signature-html|signature" --extension ts --extension tsx --extension jsRepository: Systemsaholic/tailfire
Length of output: 114
🏁 Script executed:
cat -n apps/admin/src/lib/email/build-signature-html.tsRepository: Systemsaholic/tailfire
Length of output: 2658
🏁 Script executed:
rg "buildSignatureHtml" --type ts --type tsxRepository: Systemsaholic/tailfire
Length of output: 93
🏁 Script executed:
rg "buildSignatureHtml" --type tsRepository: Systemsaholic/tailfire
Length of output: 813
🏁 Script executed:
rg "dompurify" -iRepository: Systemsaholic/tailfire
Length of output: 1660
🏁 Script executed:
cat -n apps/admin/src/lib/sanitize-email-html.tsRepository: Systemsaholic/tailfire
Length of output: 907
🏁 Script executed:
cat -n apps/admin/src/app/profile/_components/email-setup-wizard.tsx | sed -n '114,125p; 166,171p'Repository: Systemsaholic/tailfire
Length of output: 825
Sanitize signature HTML before DOM injection to prevent XSS.
User-provided tagline (via Textarea input) and profile fields are concatenated into HTML without escaping in buildSignatureHtml(), then rendered via dangerouslySetInnerHTML at line 332. An attacker can inject arbitrary HTML/JavaScript through the tagline field.
Apply sanitization to the HTML before rendering. DOMPurify is already available in the project (with a sanitizeEmailHtml utility already defined in lib/sanitize-email-html.ts):
Proposed fix
import { buildSignatureHtml } from '@/lib/email/build-signature-html'
+import { sanitizeEmailHtml } from '@/lib/sanitize-email-html'
const signaturePreviewHtml = useMemo(() => {
if (!profile) return ''
- return buildSignatureHtml({
+ return sanitizeEmailHtml(buildSignatureHtml({
firstName: profile.firstName ?? '',
lastName: profile.lastName ?? '',
designations: profile.designations,
jobTitle: profile.jobTitle,
phoneExtension: profile.phoneExtension,
avatarUrl: profile.avatarUrl,
companyName: agencyConfig?.agencyName ?? 'Phoenix Voyages',
companyPhone: agencyConfig?.companyPhone,
companyAddress: agencyConfig?.companyAddress,
ticoRegistration: agencyConfig?.ticoRegistration,
tagline: tagline || undefined,
showAvatar,
- })
+ }))
}, [profile, agencyConfig, tagline, showAvatar])🧰 Tools
🪛 ast-grep (0.42.1)
[warning] 331-331: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/app/profile/_components/email-setup-wizard.tsx` around lines
331 - 333, The signature HTML is injected via dangerouslySetInnerHTML using
signaturePreviewHtml produced by buildSignatureHtml(), which currently
concatenates user-controlled fields like tagline without escaping; fix by
importing and calling the existing sanitizeEmailHtml utility to sanitize the
HTML returned from buildSignatureHtml (e.g., const signaturePreviewHtml =
sanitizeEmailHtml(buildSignatureHtml(profile, tagline, ...))) and then use that
sanitized string in the element with dangerouslySetInnerHTML to prevent XSS.
class-validator was rejecting these new fields because they weren't declared in the DTO class. The wizard's save step was failing silently with a 400 validation error. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Domain-level server defaults are now applied server-side regardless of what the client sends. For @phoenixvoyages.ca, IMAP/SMTP host, port, and TLS settings are always set to the correct values — prevents typos like the previous 'mail.phonenixvoyages,ca' that broke email sync. Also deleted the broken account on preview so the user can re-create through the wizard. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/src/email-accounts/email-accounts.service.ts (1)
46-49: Normalize and parse the domain more defensively before lookup.Current extraction can silently miss defaults when input contains surrounding whitespace. Consider trimming once and extracting the domain via
lastIndexOf('@')to make enforcement more robust.♻️ Proposed change
- const domain = dto.emailAddress.split('@')[1]?.toLowerCase() + const normalizedEmail = dto.emailAddress.trim() + const atIndex = normalizedEmail.lastIndexOf('@') + const domain = atIndex >= 0 ? normalizedEmail.slice(atIndex + 1).toLowerCase() : undefined const domainDefaults = domain ? EmailAccountsService.DOMAIN_SERVER_DEFAULTS[domain] : undefined🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/email-accounts/email-accounts.service.ts` around lines 46 - 49, The domain extraction is fragile: instead of splitting dto.emailAddress on '@', trim the email once and locate the domain with lastIndexOf('@') to handle surrounding whitespace and multiple '@' chars; compute domain = trimmedEmail.slice(atIndex + 1).toLowerCase() only when atIndex >= 0 and domain is non-empty, then use EmailAccountsService.DOMAIN_SERVER_DEFAULTS[domain]; ensure you fall back to undefined when no valid domain is found.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/api/src/email-accounts/email-accounts.service.ts`:
- Around line 46-49: The domain extraction is fragile: instead of splitting
dto.emailAddress on '@', trim the email once and locate the domain with
lastIndexOf('@') to handle surrounding whitespace and multiple '@' chars;
compute domain = trimmedEmail.slice(atIndex + 1).toLowerCase() only when atIndex
>= 0 and domain is non-empty, then use
EmailAccountsService.DOMAIN_SERVER_DEFAULTS[domain]; ensure you fall back to
undefined when no valid domain is found.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3516d87d-4863-431b-a45c-b4870f9cfd9e
📒 Files selected for processing (1)
apps/api/src/email-accounts/email-accounts.service.ts
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
1 participant
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
Streamlines new user email setup with a focused two-step wizard and improves the profile email tab for all users.
Email Setup Wizard (
/profile?setup=true)@phoenixvoyages.ca(read-only), user only enters password. "Test & Connect" validates then creates account.signatureHtml+ setsonboardingCompletedAt.Profile Email Tab Improvements
@phoenixvoyages.caNew Data Fields
user_profiles:designations,job_title,phone_extension(added to Agent Info tab)agency_settings:company_phone,company_toll_free,company_email,company_address,tico_registration(seeded for Phoenix Voyages)getBusinessConfiguration()now reads from DB instead of hardcodingTest plan
/profile?setup=true→ wizard appears/welcomeredirect🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Chores