Skip to content

Validate automation email Lexical documents#28725

Draft
EvanHahn wants to merge 1 commit into
mainfrom
ny1284-validate-lexical-string
Draft

Validate automation email Lexical documents#28725
EvanHahn wants to merge 1 commit into
mainfrom
ny1284-validate-lexical-string

Conversation

@EvanHahn

@EvanHahn EvanHahn commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

closes https://linear.app/ghost/issue/NY-1284

Don't just check whether the JSON is valid: check that it's valid Lexical.

Manually tested this by editing an email and it worked fine.

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 967f911a-e4cb-4c0e-81b1-58eefa0f45ba

📥 Commits

Reviewing files that changed from the base of the PR and between 5148e6d and 7fdbbd4.

⛔ Files ignored due to path filters (1)
  • ghost/core/test/e2e-api/admin/__snapshots__/automations.test.js.snap is excluded by !**/*.snap
📒 Files selected for processing (6)
  • ghost/core/core/server/api/endpoints/utils/validators/input/automation_email_previews.js
  • ghost/core/core/server/lib/lexical.js
  • ghost/core/core/server/services/automations/automations-api.ts
  • ghost/core/test/unit/server/lib/lexical.test.js
  • ghost/core/test/unit/server/services/automations/automations-api.test.js
  • ghost/core/test/utils/automations-fixtures.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • ghost/core/test/unit/server/lib/lexical.test.js
  • ghost/core/test/utils/automations-fixtures.ts
  • ghost/core/core/server/api/endpoints/utils/validators/input/automation_email_previews.js
  • ghost/core/core/server/lib/lexical.js
  • ghost/core/core/server/services/automations/automations-api.ts

Walkthrough

The pull request adds a validate(lexical, userOptions) method to ghost/core/server/lib/lexical.js, backed by a new createLexicalHtmlRenderer(onError) helper that allows injecting an error-rethrowing handler into LexicalHTMLRenderer. This method is then used to replace JSON-parse–based validation in two locations: the automation_email_previews.js request validators (whose preview and sendTestEmail handlers become async) and the automations-api.ts edit service (where validateEditData becomes async and a new validateEmailLexical helper handles invalid and empty-document edge cases). The NON_EMPTY_EMAIL_LEXICAL fixture is updated to a fully specified Lexical schema, and unit tests are added for validate() and the new automations edit rejection paths.

Possibly related PRs

  • TryGhost/Ghost#28720: Introduces the automation_email_previews preview/test endpoints whose validators are updated in this PR to use lexicalLib.validate.

Suggested reviewers

  • troyciesco
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and concisely summarizes the main change: adding validation for automation email Lexical documents.
Description check ✅ Passed The description is related to the changeset, referencing the Linear issue and explaining the validation improvement from JSON-only to Lexical-aware validation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ny1284-validate-lexical-string

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nx-cloud

nx-cloud Bot commented Jun 18, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 5148e6d

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 2m 18s View ↗
nx run ghost:test:ci:integration:no-coverage ✅ Succeeded 2m 24s View ↗
nx run ghost:test:ci:legacy ✅ Succeeded 2m 52s View ↗
nx run ghost:test:ci:e2e ✅ Succeeded 2m 27s View ↗
nx run ghost:test:ci:e2e:no-coverage ✅ Succeeded 2m 11s View ↗
nx build @tryghost/portal ✅ Succeeded 1s View ↗
nx build @tryghost/signup-form ✅ Succeeded <1s View ↗
nx build @tryghost/comments-ui ✅ Succeeded <1s View ↗
Additional runs (10) ✅ Succeeded ... View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-06-22 23:16:48 UTC

@EvanHahn EvanHahn force-pushed the ny1284-validate-lexical-string branch from ae249d3 to 5148e6d Compare June 22, 2026 22:50
@EvanHahn EvanHahn marked this pull request as ready for review June 22, 2026 22:52

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ghost/core/core/server/lib/lexical.js`:
- Around line 116-124: The validate() method currently only uses userOptions
when creating the lexicalValidationRenderer, but the render() method supplies
additional Ghost defaults like siteUrl, image options, feature flags, and custom
nodeRenderers. Extract the option-building logic from the render() method (lines
90-111 in the diff context) and reuse it in the validate() method so that the
validation renderer is called with the same complete set of defaults as the
normal render() method, ensuring that valid documents depending on those
defaults are not incorrectly rejected during validation.

In `@ghost/core/core/server/services/automations/automations-api.ts`:
- Around line 117-118: The validateActiveEmailSteps function calls
isEmptyLexical internally, which catches malformed JSON errors and returns true,
causing the API to report an emptyEmailBodyWhenActive error instead of the
proper invalid-Lexical error. Swap the order of the two validation calls so that
validateEmailLexical executes before validateActiveEmailSteps, ensuring that
malformed Lexical content is properly detected and reported with the correct
error classification before the active-body emptiness check runs.
- Around line 182-193: In the isEmptyParsedLexical function, the current check
for whether a paragraph's children field is empty is too permissive. The
condition `!children[0].children || children[0].children.length === 0` treats
missing, null, or malformed children values as empty. Replace this logic to
explicitly require that children[0].children is an array before considering the
paragraph empty, so that only a valid empty array is treated as an empty
paragraph, rejecting malformed Lexical data.
🪄 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: 08cd6934-7bde-47d3-93b6-33a988c726d4

📥 Commits

Reviewing files that changed from the base of the PR and between 052c31f and 5148e6d.

⛔ Files ignored due to path filters (1)
  • ghost/core/test/e2e-api/admin/__snapshots__/automations.test.js.snap is excluded by !**/*.snap
📒 Files selected for processing (6)
  • ghost/core/core/server/api/endpoints/utils/validators/input/automation_email_previews.js
  • ghost/core/core/server/lib/lexical.js
  • ghost/core/core/server/services/automations/automations-api.ts
  • ghost/core/test/unit/server/lib/lexical.test.js
  • ghost/core/test/unit/server/services/automations/automations-api.test.js
  • ghost/core/test/utils/automations-fixtures.ts

Comment thread ghost/core/core/server/lib/lexical.js Outdated
Comment thread ghost/core/core/server/services/automations/automations-api.ts Outdated
Comment thread ghost/core/core/server/services/automations/automations-api.ts Outdated
@EvanHahn EvanHahn force-pushed the ny1284-validate-lexical-string branch from 5148e6d to 7fdbbd4 Compare June 22, 2026 23:09
@EvanHahn EvanHahn marked this pull request as draft June 22, 2026 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant