Removed unused welcome email design fields from types#27521
Merged
troyciesco merged 3 commits intomainfrom Apr 23, 2026
Merged
Conversation
The admin-x-settings email-design stack is now only used by welcome emails, so removing newsletter-only fields keeps the types and helpers aligned with the actual UI.
Contributor
WalkthroughRemoved the TitleAlignmentField component and its re-export. Deleted postTitleColor and its resolution logic from the email color model. Simplified types by removing EmailDesignPreviewSettings and merging to a single EmailDesignSettings type; DEFAULT_EMAIL_DESIGN no longer includes post_title_color or title_alignment. mapApiToDesignSettings now accepts EmailDesignSettings and filters results to keys in DEFAULT_EMAIL_DESIGN; buildAutomatedEmailDesignPayload applies the same whitelist. Unit tests were updated to disallow unexpected persisted fields (e.g., custom_future_field) from passing through. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/admin-x-settings/test/unit/email-design/design-payload.test.ts (1)
7-32:⚠️ Potential issue | 🟡 MinorSeed the removed fields before asserting they are stripped.
These assertions pass even if the helpers leak
post_title_colorortitle_alignment, because the fixtures do not contain those keys. Add legacy values so the tests exercise the cleanup path.🧪 Proposed test fixture update
const apiData = { id: '1', slug: 'default-automated-email', created_at: '2026-04-02T00:00:00.000Z', updated_at: '2026-04-02T00:00:00.000Z', header_image: null, show_header_icon: true, show_header_title: true, show_badge: true, footer_content: null, - ...DEFAULT_EMAIL_DESIGN + ...DEFAULT_EMAIL_DESIGN, + post_title_color: '#15171a', + title_alignment: 'center' }; @@ designSettings: { ...DEFAULT_EMAIL_DESIGN, id: '1', slug: 'default-automated-email', created_at: '2026-04-02T00:00:00.000Z', updated_at: '2026-04-02T00:00:00.000Z', - custom_future_field: '#abcdef' + custom_future_field: '#abcdef', + post_title_color: '#15171a', + title_alignment: 'center' } as unknown as typeof DEFAULT_EMAIL_DESIGN & { id: string; slug: string; created_at: string; updated_at: string; custom_future_field: string; + post_title_color: string; + title_alignment: string; },Also applies to: 47-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-x-settings/test/unit/email-design/design-payload.test.ts` around lines 7 - 32, The test is asserting that legacy fields are removed but doesn't seed those keys, so update the test fixture in apps/admin-x-settings/test/unit/email-design/design-payload.test.ts by adding legacy fields (e.g., post_title_color and title_alignment) to the apiData object (alongside the already-present id/slug/created_at/updated_at/header_image/show_header_icon/show_header_title/show_badge/footer_content and DEFAULT_EMAIL_DESIGN) before calling mapApiToDesignSettings, and do the same for the other test block (lines 47-88) so mapApiToDesignSettings is actually exercised to strip those legacy properties.
🤖 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-x-settings/src/components/settings/membership/member-emails/welcome-email-customize-modal.tsx`:
- Around line 312-319: The code is still including legacy newsletter-only fields
(post_title_color, title_alignment) when building persistedDesign and when
filtering apiData; update the filtering used in both the function that builds
the initial design object and buildAutomatedEmailDesignPayload (where
persistedDesign is created) to also exclude 'post_title_color' and
'title_alignment' — either add those keys to the NON_DESIGN_FIELDS set or extend
the filter predicate to return false for those exact keys so they are not
hydrated or resent.
---
Outside diff comments:
In `@apps/admin-x-settings/test/unit/email-design/design-payload.test.ts`:
- Around line 7-32: The test is asserting that legacy fields are removed but
doesn't seed those keys, so update the test fixture in
apps/admin-x-settings/test/unit/email-design/design-payload.test.ts by adding
legacy fields (e.g., post_title_color and title_alignment) to the apiData object
(alongside the already-present
id/slug/created_at/updated_at/header_image/show_header_icon/show_header_title/show_badge/footer_content
and DEFAULT_EMAIL_DESIGN) before calling mapApiToDesignSettings, and do the same
for the other test block (lines 47-88) so mapApiToDesignSettings is actually
exercised to strip those legacy properties.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1bd7e345-1439-4a1c-8371-e641b59df485
📒 Files selected for processing (6)
apps/admin-x-settings/src/components/settings/email-design/design-fields/index.tsapps/admin-x-settings/src/components/settings/email-design/design-fields/title-alignment-field.tsxapps/admin-x-settings/src/components/settings/email-design/design-utils.tsapps/admin-x-settings/src/components/settings/email-design/types.tsapps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-customize-modal.tsxapps/admin-x-settings/test/unit/email-design/design-payload.test.ts
💤 Files with no reviewable changes (3)
- apps/admin-x-settings/src/components/settings/email-design/design-fields/index.ts
- apps/admin-x-settings/src/components/settings/email-design/design-fields/title-alignment-field.tsx
- apps/admin-x-settings/src/components/settings/email-design/design-utils.ts
The removed newsletter-only field names were only being referenced in negative assertions, which kept legacy details in the test without improving coverage for the current welcome email design shape.
Using an allowlist based on the current welcome email design shape makes the helpers stricter and avoids carrying forward unexpected keys from future response or state changes.
|
cmraible
added
the
ok to merge for me
cmraible
changed the title
troyciesco
approved these changes
Apr 23, 2026
troyciesco
deleted the
chris-ny-1204-clean-up-post_title_alignment-and-title_color-attributes
branch
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.
ref https://linear.app/ghost/issue/NY-1204/clean-up-post-title-alignment-and-title-color-attributes
This should have no user-facing impact.
These fields snuck into the original implementation, but ended up not being used for welcome emails. These are newsletter-only fields. Since the email-design component currently is only used by welcome emails, this cleans up the current types to match the current UI.