chore(messaging): drop vestigial from.email and from.name in native email#60438
Merged
Conversation
…later The native email integration choice read `integration.config.email_address` (a field that does not exist on email integrations) and fell back to `'default@example.com'`. The actual fields stored on the integration config are `email` and `name` (see `posthog/models/integration.py` and `emailSetupModalLogic.ts`), so the stored form value was always the dummy default. These values are functionally vestigial — the email service overwrites `params.from.email` and `params.from.name` from the integration config at send time in `validateEmailDomain` — but the form still surfaces them through frontend validation and the zod schema, so populating them correctly is the right behavior. This also enables the previously commented-out `name` so the form data matches what the integration actually provides.
Contributor
|
Size Change: 0 B Total Size: 80.7 MB ℹ️ View Unchanged
|
…mail
The `from` object in `native_email` inputs carried `email` and `name`
alongside `integrationId`, but both were functionally dead. At send time
`email.service.ts` overwrote them from `integration.config.email` and
`integration.config.name`, so the form-stored values never reached the
recipient. They only existed to satisfy the zod schema and frontend
validation.
This commit removes them:
- `nodejs/src/schema/cyclotron.ts`: `from` is now `{ integrationId: number }`
- `nodejs/src/cdp/services/messaging/email.service.ts`: the renamed
`resolveFromSender(integration)` returns `{ email, name }` from the
integration config, and both maildev/SES send paths take it as a local
parameter instead of mutating `params.from`.
- `nodejs/src/cdp/templates/_destinations/email/email.template.ts`:
default `from` is now `{}`; the user fills it in by picking an
integration sender.
- `frontend/src/scenes/hog-functions/email-templater/EmailTemplater.tsx`:
`NativeEmailIntegrationChoice` only emits `{ integrationId }`.
- `products/workflows/frontend/Workflows/workflowLogic.ts`: the email
step `from` validation only checks `integrationId`; the templating
error check on `from.email` is gone (the field no longer exists).
- Tests updated accordingly. The
`email.service.test.ts` "should ignore a given email and use the
integration config" case is removed — there is no longer a way to
pass an email through the schema for the service to ignore.
Pre-existing rows with `from.email`/`from.name` keep working: zod strips
unknown keys, and the new frontend validation only requires
`integrationId`, which existing valid rows already have.
dmarchuk
changed the title
…ctor
After dropping `from.email` from the email step validation, the two
"no integrationId" tests (one with `from: {}`, one with
`from: undefined`) both exercise the same `!emailValue?.from?.integrationId`
check via the same falsy path. Combined them into a parameterized
`it.each` so the intent stays visible without duplicating setup.
Contributor
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
Contributor
|
Reviews (1): Last reviewed commit: "Merge branch 'master' into claude/tender..." | Re-trigger Greptile |
meikelmosby
approved these changes
Jun 1, 2026
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
3 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.
Problem
Follow-up cleanup on the native email templater PR (#35984). The
fromobject onnative_emailinputs carried three keys —integrationId,email,name— but onlyintegrationIdactually mattered.Trace:
NativeEmailIntegrationChoice.onChangeIntegrationpopulatedemailby readingintegration.config.email_address, a field that does not exist on the email integration config (the real key isemail; seeposthog/models/integration.py:2020andemailSetupModalLogic.ts:57). So the stored value was always the'default@example.com'fallback.namewas commented out entirely (// TODO: Add support for the name?).validateEmailDomaininnodejs/src/cdp/services/messaging/email.service.tsoverwrote bothparams.from.emailandparams.from.namefromintegration.configbefore reading them. The form-stored values never reached the recipient.So
emailandnameonly existed to satisfy the zod schema and the frontendfrom-field validation. We can drop them.Changes
nodejs/src/schema/cyclotron.ts:fromis now{ integrationId: number }.nodejs/src/cdp/services/messaging/email.service.ts:validateEmailDomainis renamed toresolveFromSender(integration)and returns{ email, name }fromintegration.configinstead of mutatingparams.from. Maildev and SES send paths take the resolved sender as a local parameter.nodejs/src/cdp/templates/_destinations/email/email.template.ts: defaultfromis now{}. The user picks an integration sender to fill it in.frontend/.../EmailTemplater.tsx:NativeEmailIntegrationChoiceonly emits{ integrationId }.products/workflows/.../workflowLogic.ts: the email stepfromvalidation only checksintegrationId; the templating-error check onfrom.emailis gone because the field no longer exists.email.service.test.ts"should ignore a given email and use the integration config" is removed — there is no longer a way to pass an email through the schema for the service to ignore.Backward compatibility
Pre-existing rows with
from.email/from.namekeep working. Zod strips unknown keys when parsing, and the new frontend validation only requiresintegrationId, which existing valid rows already have.How did you test this code?
I tested this locally with both maildev and actual SES setup to make sure we still get the emails with the right configuration.
I'm an agent. I ran:
pnpm exec jest --testPathPattern="email-templater"→ 4 tests pass (emailTemplaterLogic.test.ts).pnpm exec jest src/cdp/services/messaging/email.service.test.ts(nodejs) → the 17 pure-unit tests forsanitizeEmailSubject/parseAddressListpass; the 25 integration tests fail in this sandbox because Postgres/Kafka aren't running, so CI will be the source of truth for those.pnpm exec jest --testPathPattern="emailValidation"(frontend) → file fails to load due to a pre-existing@posthog/quillmodule-resolution issue that also reproduces onmasterwithout my changes. CI will run it cleanly.tsc --noEmit) shows no new errors in the files I touched.I did not exercise the UI manually.
Publish to changelog?
no
🤖 Agent context
email: integration?.config?.email_address ?? 'default@example.com'TODO and the commented-outnamein the newNativeEmailIntegrationChoiceand asked whether either is needed.email_address→email, plus enablingname) because the structural removal touches the queue schema, validation, and tests, and I wasn't confident I could do it cleanly. The user pushed back ("why would I do a cleanup in 2 PRs?") so this revision finishes the job in this same PR —fromis now just{ integrationId }end-to-end, and the email/name flow throughresolveFromSenderas a local value inside the email service rather than getting smuggled through the queue payload.email, there is no longer a path for a user-supplied email to reach the service at all, so the test no longer expresses anything meaningful. The intent it captured (the integration's email is authoritative) is now structural rather than runtime.