diff --git a/admin/src/app/auth/LoginPage.tsx b/admin/src/app/auth/LoginPage.tsx index b5512a8e..1faadb82 100644 --- a/admin/src/app/auth/LoginPage.tsx +++ b/admin/src/app/auth/LoginPage.tsx @@ -21,7 +21,7 @@ import { Label } from "@/components/ui/label"; import { InputOTP, InputOTPGroup, InputOTPSlot } from "@/components/ui/input-otp"; import { Logo } from "@/components/Logo"; import { TurnstileModal } from "@/components/captcha/TurnstileModal"; -import { login, loginConfirm, verifyTwoFA } from "@/lib/api/client/auth"; +import { getAuthConfig, login, loginConfirm, verifyTwoFA } from "@/lib/api/client/auth"; import type { LoginResponse } from "@/lib/api/models/auth"; import { setToken } from "@/lib/auth/storage"; import { APIError } from "@/lib/api/client"; @@ -51,6 +51,12 @@ export default function LoginPage() { const [session, setSession] = useState(""); const [code, setCode] = useState(""); const [captcha, setCaptcha] = useState(false); + // Whether this deployment verifies a captcha token at all. null until the + // answer arrives: nothing is mounted before then, so an instance with no + // route to Cloudflare does not raise a widget error on a screen nobody has + // submitted. A failed fetch resolves to true rather than leaving it + // pending, so the check is never skipped on an instance that enforces it. + const [captchaRequired, setCaptchaRequired] = useState(null); const [submitting, setSubmitting] = useState(false); const [error, setError] = useState(null); const [resendIn, setResendIn] = useState(0); @@ -59,6 +65,24 @@ export default function LoginPage() { const busy = submitting || captcha; + // What this deployment can do, read before anything mounts a widget. A + // self-host with CAPTCHA_PROVIDER=none cannot reach Cloudflare, so an + // invisible Turnstile there can only time out and lock the operator out. + useEffect(() => { + let cancelled = false; + getAuthConfig() + .then((cfg) => { + if (!cancelled) setCaptchaRequired(cfg.captcha); + }) + .catch(() => { + // Fail safe: assume the check is enforced. + if (!cancelled) setCaptchaRequired(true); + }); + return () => { + cancelled = true; + }; + }, []); + // Resend cooldown tick. useEffect(() => { if (resendIn <= 0) return; @@ -311,7 +335,12 @@ export default function LoginPage() { )} - + ) : ( diff --git a/admin/src/components/captcha/TurnstileModal.tsx b/admin/src/components/captcha/TurnstileModal.tsx index 5c63e1f6..9d1d195f 100644 --- a/admin/src/components/captcha/TurnstileModal.tsx +++ b/admin/src/components/captcha/TurnstileModal.tsx @@ -5,6 +5,20 @@ // the backend's TURNSTILE_BYPASS_TOKEN accepts; in prod it renders the // invisible Turnstile widget and delivers a real token. Either way the parent // gets a token via onToken and sends it as `turnstile` on login. +// +// `required` is the deployment's own answer, read from /v1/auth/config. With +// CAPTCHA_PROVIDER=none the backend verifies no token, and mounting the widget +// anyway meant an air-gapped or self-hosted instance could only sit on +// challenges.cloudflare.com until it timed out: the operator was locked out of +// their own admin panel. +// +// null means the answer has not arrived. Nothing is mounted and no token is +// delivered while it is pending, because mounting optimistically loads the +// Cloudflare script on an instance that may have no route to it, and its load +// failure raises an error on a screen the operator has not even submitted yet. +// A submit made in that window is held by the effect and resolves as soon as +// the config lands. A fetch that fails resolves to true, so the check is never +// skipped on an instance that does enforce it. import { useCallback, useEffect, useRef, type ComponentProps } from "react"; import Turnstile, { type BoundTurnstileObject } from "react-turnstile"; @@ -12,15 +26,22 @@ import { TURNSTILE_KEY } from "@/lib/env"; interface Props { visible: boolean; + required: boolean | null; onToken: (token: string) => void; onError?: (message?: string) => void; } -export function TurnstileModal({ visible, onToken, onError }: Props) { +export function TurnstileModal({ visible, required, onToken, onError }: Props) { const defaultDevBypassToken = "warmbly-local-turnstile-bypass"; - const bypassToken = import.meta.env.DEV + const devBypassToken = import.meta.env.DEV ? import.meta.env.VITE_TURNSTILE_BYPASS_TOKEN?.trim() || defaultDevBypassToken : ""; + // The dev bypass is its own answer and needs no deployment config. + const pending = required === null && devBypassToken === ""; + // No widget, and the token the parent gets is whatever the backend will + // accept: the dev bypass string, or "" when nothing is verified at all. + const skipWidget = required === false || devBypassToken !== ""; + const bypassToken = devBypassToken; const tokenRef = useRef(""); const waitingRef = useRef(false); @@ -77,7 +98,10 @@ export function TurnstileModal({ visible, onToken, onError }: Props) { }, [fail]); useEffect(() => { - if (visible && bypassToken) { + // Hold a submit made before the deployment answered; this effect runs + // again the moment it does, with visible still true. + if (pending) return; + if (visible && skipWidget) { onTokenRef.current(bypassToken); return; } @@ -96,9 +120,9 @@ export function TurnstileModal({ visible, onToken, onError }: Props) { } waitingRef.current = false; } - }, [visible, bypassToken, deliver, execute]); + }, [visible, pending, skipWidget, bypassToken, deliver, execute]); - if (bypassToken) return null; + if (pending || skipWidget) return null; const turnstileProps = { ref: turnstileRef, diff --git a/admin/src/lib/api/client/auth/index.ts b/admin/src/lib/api/client/auth/index.ts index 3a2262c4..0e79befc 100644 --- a/admin/src/lib/api/client/auth/index.ts +++ b/admin/src/lib/api/client/auth/index.ts @@ -4,6 +4,7 @@ import { Request } from "@/lib/api/client"; import type { + DeploymentAuthConfig, LoginRequest, LoginStartResponse, LoginConfirmRequest, @@ -13,6 +14,17 @@ import type { AdminProfile, } from "@/lib/api/models/auth"; +// What this deployment's auth can do. Public and unauthenticated: it is the +// first request the sign-in screen makes, and the answer decides whether a +// Turnstile widget is mounted at all. +export function getAuthConfig(): Promise { + return Request({ + method: "GET", + url: "/v1/auth/config", + timeout: 10_000, + }); +} + // Step 1: verify password + captcha. Emails a one-time code, returns a session. export function login(input: LoginRequest): Promise { return Request({ diff --git a/admin/src/lib/api/models/auth.ts b/admin/src/lib/api/models/auth.ts index 9d7c0151..ec6fc24d 100644 --- a/admin/src/lib/api/models/auth.ts +++ b/admin/src/lib/api/models/auth.ts @@ -2,6 +2,17 @@ import type { AdminToken } from "@/lib/auth/storage"; +// The slice of GET /v1/auth/config the admin sign-in needs. +// +// Only `captcha` today, and it exists because the login screen used to guess. +// A self-host with CAPTCHA_PROVIDER=none verifies no token, but the screen +// mounted the invisible Turnstile widget anyway and waited on a challenge it +// could not reach, so the operator was locked out of their own instance with +// "Verification timed out". +export interface DeploymentAuthConfig { + captcha: boolean; +} + export interface LoginRequest { email: string; password: string; diff --git a/cmd/backend/main.go b/cmd/backend/main.go index 32068d7c..92a5dcb3 100644 --- a/cmd/backend/main.go +++ b/cmd/backend/main.go @@ -1810,9 +1810,9 @@ func main() { systemChecker.Add("redis", func(ctx context.Context) error { return cache.Ping(ctx).Err() }) switch bus.Name() { case "kafka": - systemChecker.Add("kafka", sysstatus.TCPCheck(kafkaBootstrapServers)) + systemChecker.Add("kafka", sysstatus.TCPCheck(kafkaBootstrapServers, "9092")) case "nats": - systemChecker.Add("nats", sysstatus.TCPCheck(strings.TrimPrefix(getenvDefault("NATS_URL", "nats://localhost:4222"), "nats://"))) + systemChecker.Add("nats", sysstatus.TCPCheck(getenvDefault("NATS_URL", "nats://localhost:4222"), "4222")) } if sr := os.Getenv("SCHEMA_REGISTRY_URL"); sr != "" { systemChecker.Add("schema-registry", sysstatus.HTTPCheck(strings.TrimRight(sr, "/")+"/subjects")) diff --git a/docs/content/docs/api/error-codes.mdx b/docs/content/docs/api/error-codes.mdx index 7159b92f..4075eeb2 100644 --- a/docs/content/docs/api/error-codes.mdx +++ b/docs/content/docs/api/error-codes.mdx @@ -84,6 +84,7 @@ Returned when the request cannot be processed due to invalid syntax. | `no_contacts` | `POST /contacts/verification` selected no contacts: neither `contacts` nor a `campaign_id` with refused leads | | `list_bounce_risk` | `POST /campaigns/:id/start` refused the launch on the list's projected bounce rate. Clean or verify the list, or repeat the request with `acknowledge_list_risk: true` | | `leads_undeliverable` | `POST /campaigns/:id/start` found nothing to send because address verification refused every remaining lead; the campaign is parked at `paused_undeliverable` until they are re-verified or marked deliverable | +| `empty_step_body` | `POST /campaigns/:id/start` found an email step with nothing in either body, so it would send a blank message to every lead it reached. Write the step's body and start again | | `no_leads` | `POST /campaigns/:id/start` on a campaign that has never had a lead, with `continuous` off. Add contacts, or set `continuous` so it starts empty and waits for them. A campaign whose leads have all finished is a different case: it starts and waits | | `no_remaining_leads` | A platform-initiated restart of a campaign with nothing left to send and `continuous` off found nothing to do; the campaign is `completed` again. A start you request never answers this: it turns `continuous` on and waits | | `no_organization` | The request needs a workspace and the caller has none selected. Every entitlement, limit and suppression rule is scoped to a workspace, so a write that would run unscoped is refused rather than run without those checks. API keys always carry their workspace; a dashboard session picks one at sign-in, so this normally means the session predates the workspace being chosen. Select a workspace and retry | diff --git a/docs/content/docs/api/reference/campaigns.mdx b/docs/content/docs/api/reference/campaigns.mdx index 4b640d45..d8e2b412 100644 --- a/docs/content/docs/api/reference/campaigns.mdx +++ b/docs/content/docs/api/reference/campaigns.mdx @@ -177,7 +177,7 @@ Create a campaign. Only `name` is required, every other field is optional and ap | `schedule_windows` | array | no | Per-day sending windows, 7 arrays indexed by weekday (Sunday = 0) of `{start, end}` minute-of-day intervals. When non-empty it supersedes `days`, `start_time` and `end_time`. | | `email_tag_ids` | string[] | no | Mailbox tag ids that resolve the sender pool (tags strategy). | | `folder_ids` | string[] | no | Folder ids to file the campaign under. | -| `sender_strategy` | string | no | `tags` (default) or `explicit`. | +| `sender_strategy` | string | no | `tags` (default) or `explicit`. `tags` resolves the pool from `email_tag_ids`, and falls back to every active mailbox in the workspace when no tag and no explicit sender is set. `explicit` sends from the mailboxes in `senders`, plus any `email_tag_ids` set alongside them. It never falls back to every active mailbox, so a pool that empties out parks the campaign at `paused_no_accounts` instead of widening to the whole workspace. | | `rotation_mode` | string | no | How volume spreads across the chosen mailboxes. | | `senders` | object[] | no | Explicit-strategy mailbox pool (see sender input below). | | `ramp_enabled` | boolean | no | Enable per-campaign daily ramp-up. | @@ -732,6 +732,8 @@ Start (activate) the campaign so it begins sending real mail. Works from `draft` A start refused because every remaining lead was refused by address verification answers `leads_undeliverable` and parks the campaign at `paused_undeliverable`; re-verify the leads or mark them deliverable with [`POST /contacts/verification`](/api/reference/contacts/#verify-or-override-contacts), which resumes it. +A campaign whose email step has nothing in either body is refused with `empty_step_body` rather than started, because it would send a blank message to every lead it reached. + ### Response ```json @@ -832,7 +834,7 @@ Atomically replace the campaign's explicit sender pool with the supplied list. * | Field | Type | Required | Description | | --- | --- | --- | --- | -| `senders` | object[] | yes | The full new sender pool. Each item: `email_account_id` (uuid, required), `weight` (integer, optional), `enabled` (boolean, optional). | +| `senders` | object[] | yes | The full new sender pool. Each item: `email_account_id` (uuid, required), `weight` (integer, optional), `enabled` (boolean, optional). An empty array clears the pool. What that means depends on the campaign's `sender_strategy`: a `tags` campaign falls back to its tags or to every active mailbox, while an `explicit` one falls back to its tags only, and with none it is left with no mailbox to send from and parks itself at `paused_no_accounts`. | ```json { @@ -1020,7 +1022,7 @@ All fields optional. | `name` | string | no | Step label. | | `subject` | string | no | Subject template. | | `body_plain` | string | no | Plain-text body template. Leave it empty and the send path renders one from `body_html`, keeping list bullets, table rows and link destinations, and leaving out the stylesheet. | -| `body_html` | string | no | HTML body template. Sent as written, including a whole document with its own ``. Any `", false}, + {"real copy", "
Hi Ana
", true}, + {"image only", `
`, true}, + {"image with no source or alt", "
", false}, + {"rule", "
", true}, + } + for _, c := range cases { + if got := HasContent(c.body); got != c.want { + t.Errorf("%s: HasContent(%q) = %v, want %v", c.name, c.body, got, c.want) + } + } +} + +// The round trip the derivation relies on: whatever FromText writes for a +// non-empty body must read back as content, or the send-time guard would drop +// the part it just derived. +func TestFromTextOutputHasContent(t *testing.T) { + for _, in := range []string{"Hello", "a\n\nb", "https://example.com"} { + if !HasContent(FromText(in)) { + t.Errorf("FromText(%q) produced a body HasContent calls empty", in) + } + } +} + +// The send path renders the body with text/template, which escapes nothing by +// design, so an anchor built around a templated URL would let a contact value +// containing a quote break out of the href. Such a URL stays plain text. +func TestFromTextDoesNotAnchorATemplatedURL(t *testing.T) { + got := FromText("Book here: https://cal.example.com/?ref={{.Company}}") + if strings.Contains(got, " 0 || tagCount > 0 { return nil } + // A campaign that named its mailboxes by hand does NOT fall back to every + // active mailbox (see ExplicitSenderPool), so the check must not either: + // a check that passes a pool the scheduler would find empty is how a + // campaign starts and then parks itself on its first tick (issue #340 is + // the same mistake in the other direction). + var strategy string + if err := r.DB.QueryRow(ctx, `SELECT sender_strategy FROM campaigns WHERE id = $1`, campaignID).Scan(&strategy); err != nil { + return err + } + if strategy == CampaignSenderStrategyExplicit { + return errx.New(errx.BadRequest, + "this campaign sends from mailboxes picked by hand and none are left; pick its sending accounts again, or switch it back to selecting by tag") + } var activeMailboxes int if err := r.DB.QueryRow(ctx, ` SELECT COUNT(*) FROM email_accounts @@ -1979,15 +2001,15 @@ func (r *campaignRepository) GetCampaignSenders(ctx context.Context, campaignID return senders, nil } -// ReplaceCampaignSenders atomically swaps the explicit sender pool. An empty -// list is rejected — clearing senders should be done by switching the campaign -// back to sender_strategy='tags'. +// ReplaceCampaignSenders atomically swaps the explicit sender pool. +// +// An empty list is allowed and clears the pool: it is how the dashboard's +// sending-accounts picker deselects everything. What that then means depends +// on the campaign's sender_strategy, which is the safety boundary — a 'tags' +// campaign falls back to its tags or to every active mailbox, and an +// 'explicit' one resolves to no mailboxes at all rather than widening to the +// whole workspace (see ExplicitSenderPool). func (r *campaignRepository) ReplaceCampaignSenders(ctx context.Context, campaignID uuid.UUID, in []models.CampaignSenderInput) ([]models.CampaignSender, *errx.Error) { - // An empty list is allowed: it clears the explicit sender pool, so the - // campaign falls back to its email tags or, with neither, to every active - // mailbox of the owner. syncCampaignSendersTx handles the empty set safely - // (it deletes all current rows and inserts none). - // Resolve the campaign owner + organization so we can validate mailbox // ownership against the org (the senders route is org-scoped). var userID string diff --git a/internal/repository/pg_sequence.go b/internal/repository/pg_sequence.go index 01b2de71..82534690 100644 --- a/internal/repository/pg_sequence.go +++ b/internal/repository/pg_sequence.go @@ -16,6 +16,12 @@ import ( "github.com/warmbly/warmbly/internal/pkg/encrypt" ) +// emptyBodyHTML is what a step with no body carries: the dashboard composer's +// own empty document, so opening a blank step in the editor shows an empty +// canvas rather than nothing at all. It is NOT an empty string, which is why +// every "does this step have a body" check goes through mailhtml.HasContent. +const emptyBodyHTML = "
" + type SequenceRepository interface { Create(ctx context.Context, userID, campaignID string) (*models.Sequence, *errx.Error) Get(ctx context.Context, userID, campaignID string) ([]models.Sequence, *errx.Error) @@ -194,7 +200,7 @@ func (r *sequenceRepository) Create(ctx context.Context, userID string, campaign config.SequenceDefaultName, "", "", - "
", + emptyBodyHTML, nextPos, } diff --git a/internal/tasks/campaign_task.go b/internal/tasks/campaign_task.go index 345fbbcf..16e85b27 100644 --- a/internal/tasks/campaign_task.go +++ b/internal/tasks/campaign_task.go @@ -567,6 +567,10 @@ func (s *tasksService) HandleCampaignTask(task *proto.ProcessTask) *errx.Error { bodyHTML = "" } + // A blank HTML alternative never ships as the part the client prefers. + // Shared with the preview and the test send (see dropBlankHTMLPart). + bodyHTML = dropBlankHTMLPart(bodyHTML, bodyPlain) + // STEP 10.7: A hand-placed {{.UnsubscribeLink}} resolved to the bare signed // URL; give it an anchor so the recipient reads "Unsubscribe" and not the // API address (issue #341). After the plain part was derived, so plain text @@ -1066,16 +1070,56 @@ func (s *tasksService) clearIdle(ctx context.Context, campaign *models.Campaign) // reason is carried through to the activity log because "paused_no_accounts" // covers several very different fixes (connect a mailbox, widen a sending // window, repair DNS) and the status alone cannot tell them apart. +// +// It is deliberately LOUD. This runs unattended, hours after anyone touched +// the campaign, and a pause nobody is told about is a campaign that quietly +// stops sending: an error-level line in the operator's log, an error-level +// entry in the activity feed so the dashboard tints it red, and an org-scoped +// realtime pulse so every teammate's campaign list moves to "paused — no +// accounts" without a refresh. func (s *tasksService) autoPauseCampaign(ctx context.Context, campaignID, taskID uuid.UUID, reason string) { s.campaignRepo.UpdateStatusWithLock(ctx, campaignID, "paused_no_accounts") - s.taskRepo.UpdateTaskStatus(ctx, taskID, "completed") - if s.campaignLogRepo != nil { - s.campaignLogRepo.CreateLog(ctx, &repository.CampaignLogEntry{ - CampaignID: campaignID, - EventType: "auto_paused", - Message: reason, - }) + if taskID != uuid.Nil { + s.taskRepo.UpdateTaskStatus(ctx, taskID, "completed") } + + log.Error().Str("campaign_id", campaignID.String()).Str("reason", reason). + Msg("campaign auto-paused: no mailbox can send for it") + + // CreateLogOnce, not CreateLog: the reconciler re-checks paused campaigns + // too, and a repeat of the SAME reason must not fill the feed or re-pulse + // the dashboard every pass. Keyed on the reason rather than the code, so a + // pause whose cause changed (DNS, then no mailbox at all) still says so. + // The write also reports whether this pause is news, which is what gates + // the announcement below. + if s.campaignLogRepo == nil { + return + } + entry := &repository.CampaignLogEntry{ + CampaignID: campaignID, + EventType: "auto_paused", + Message: reason, + Metadata: map[string]interface{}{ + "level": "error", + "code": "no_accounts", + "reason": reason, + }, + } + written, err := s.campaignLogRepo.CreateLogOnce(ctx, entry, "reason", reason, time.Now().Add(-time.Hour)) + if err != nil || !written || s.streamingPublisher == nil { + return + } + campaign, gerr := s.campaignRepo.GetByID(ctx, campaignID) + if gerr != nil || campaign == nil { + return + } + s.streamingPublisher.PublishCampaignEvent(ctx, &pubsub.CampaignEvent{ + BaseEvent: pubsub.BaseEvent{EventType: pubsub.EventCampaignPaused, UserID: campaign.UserID}, + OrgID: campaignOrgID(campaign), + CampaignID: campaignID.String(), + Name: campaign.Name, + Status: "paused_no_accounts", + }) } // haltOrglessCampaign stops a campaign that reached the send path with no diff --git a/internal/tasks/preview.go b/internal/tasks/preview.go index 00bea8a8..7dce1df7 100644 --- a/internal/tasks/preview.go +++ b/internal/tasks/preview.go @@ -92,6 +92,26 @@ func (s *tasksService) PreviewEmail(ctx context.Context, orgID uuid.UUID, in Ema return out } +// dropBlankHTMLPart removes an HTML alternative that would render nothing. +// +// An empty HTML part is worse than no HTML part: every modern client prefers +// text/html, so the recipient reads a blank message while the real copy sits +// unread in the text alternative. It catches whatever produced the row, which +// is the composer's
placeholder on an API-created step, but also a +// variant whose HTML-only spintax resolved away. +// +// With no plain part either there is nothing to fall back to, so the body is +// left alone; a campaign in that state is refused at start instead. +func dropBlankHTMLPart(bodyHTML, bodyPlain string) string { + if bodyHTML == "" || strings.TrimSpace(bodyPlain) == "" { + return bodyHTML + } + if mailhtml.HasContent(bodyHTML) { + return bodyHTML + } + return "" +} + // finishBody applies what the send path adds after rendering, in its order: // derive the plain part, drop HTML for a plain-text campaign, turn a // hand-placed unsubscribe link into an anchor, add the mailbox signature, @@ -104,6 +124,9 @@ func finishBody(bodyHTML, bodyPlain string, textOnly bool, account *models.Email if textOnly { bodyHTML = "" } + // Shared with the send path, so the preview and the test send show the + // same message a recipient gets. + bodyHTML = dropBlankHTMLPart(bodyHTML, bodyPlain) // After the plain part is derived, so plain text keeps the URL it needs. linkText := "" if optOut != nil { diff --git a/internal/tasks/preview_test.go b/internal/tasks/preview_test.go index ced6bc70..d8bfe909 100644 --- a/internal/tasks/preview_test.go +++ b/internal/tasks/preview_test.go @@ -53,3 +53,38 @@ func TestPreviewTemplatesWithUsesTheGivenLink(t *testing.T) { t.Fatalf("default preview link changed: %q", q.BodyHTML) } } + +// A step created through the API or an agent tool carries the composer's empty +//
under body_html. It is not an empty string, so it shipped as the +// text/html alternative and the recipient opened a blank message with the real +// copy only in the fallback part. +func TestFinishBodyDropsATextlessHTMLPart(t *testing.T) { + h, p := finishBody("
", "Hi Ana, quick question.", false, nil, nil, "") + if h != "" { + t.Errorf("HTML part = %q, want it dropped so the plain part is what ships", h) + } + if p != "Hi Ana, quick question." { + t.Errorf("plain part = %q, want the step's own text", p) + } +} + +// The guard must not touch a body that renders something, including one whose +// only content is an image. +func TestFinishBodyKeepsABodyThatRenders(t *testing.T) { + if h, _ := finishBody("
Hi
", "Hi", false, nil, nil, ""); h != "
Hi
" { + t.Errorf("HTML part = %q, want it kept", h) + } + img := `
` + if h, _ := finishBody(img, "see the image", false, nil, nil, ""); h == "" { + t.Error("an image-only body was dropped; it renders for the recipient") + } +} + +// With no plain part either there is nothing to fall back to, so dropping the +// HTML would send an empty message. The start-time guard is what stops that +// campaign existing; here the body is left alone. +func TestFinishBodyKeepsAnEmptyHTMLPartWhenThereIsNoPlainText(t *testing.T) { + if h, _ := finishBody("
", "", false, nil, nil, ""); h == "" { + t.Error("dropped the only part the message had") + } +} diff --git a/web/src/components/app/campaigns/NewCampaignDialog.tsx b/web/src/components/app/campaigns/NewCampaignDialog.tsx index 120bf6f9..3db003fa 100644 --- a/web/src/components/app/campaigns/NewCampaignDialog.tsx +++ b/web/src/components/app/campaigns/NewCampaignDialog.tsx @@ -354,6 +354,12 @@ export function NewCampaignDialog({ open, onClose }: Props) { return () => document.removeEventListener("keydown", onKey); }, [open, requestClose]); + // The wizard writes plain text, so it sends plain text. The backend renders + // the HTML part from it: it used to be built here with an escapeHtml that + // turned the quotes in a conditional ({{if eq .Company "Acme"}}) into + // entities, which makes the template fail to parse at send time and ships + // the literal {{if}} to the recipient. The server's version also links bare + // URLs, so a wizard-written step gets click tracking like any other. function buildSteps() { return draft.sequences .filter((s) => s.subject.trim().length > 0 || s.body_plain.trim().length > 0) @@ -361,7 +367,6 @@ export function NewCampaignDialog({ open, onClose }: Props) { name: draft.kind === "one_time" ? "Email" : `Step ${i + 1}`, subject: s.subject.trim(), body_plain: s.body_plain, - body_html: `
${escapeHtml(s.body_plain).replace(/\n/g, "
")}
`, wait_after: i === 0 ? 0 : Math.max(0, s.wait_after), })); } @@ -1476,11 +1481,3 @@ function EstimatePanel({ ); } -function escapeHtml(s: string): string { - return s - .replace(/&/g, "&") - .replace(//g, ">") - .replace(/"/g, """) - .replace(/'/g, "'"); -} diff --git a/web/src/lib/api/client/app/campaigns/createCampaign.ts b/web/src/lib/api/client/app/campaigns/createCampaign.ts index cf6ad8da..a2c28daf 100644 --- a/web/src/lib/api/client/app/campaigns/createCampaign.ts +++ b/web/src/lib/api/client/app/campaigns/createCampaign.ts @@ -59,7 +59,10 @@ export interface CreateCampaignInput { name: string; subject: string; body_plain: string; - body_html: string; + // Optional: leave it off for a plain-text step and the backend renders + // the HTML part from body_plain, keeping the line structure and turning + // bare URLs into links so click tracking has hrefs to wrap. + body_html?: string; body_sync?: boolean; body_code?: boolean; wait_after?: number;