Skip to content

Updated warnings and errors for automations email steps#28364

Merged
troyciesco merged 16 commits into
mainfrom
NY-1301_automations-errs
Jun 4, 2026
Merged

Updated warnings and errors for automations email steps#28364
troyciesco merged 16 commits into
mainfrom
NY-1301_automations-errs

Conversation

@troyciesco
Copy link
Copy Markdown
Contributor

@troyciesco troyciesco commented Jun 4, 2026

closes NY-1301

What changed

  • Added a confirmation dialog before deleting send email steps from the Automations canvas.
  • Kept the existing immediate deletion behavior for non-email steps.
  • allows saving automations with send_email steps that have an empty body/subject, but does not allow publishing the automation until those fields are filled in.
  • Added unit coverage for cancelling and confirming email deletion.

This replaces #28309 but has a few extra pieces of cleanup and fixes merge conflicts from other automations changes

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 4, 2026

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: 7a4f1f8f-a176-475a-a68d-5394c29178d3

📥 Commits

Reviewing files that changed from the base of the PR and between 1ed775f and fce88d1.

📒 Files selected for processing (1)
  • apps/posts/src/views/Automations/editor.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/posts/src/views/Automations/editor.tsx

Walkthrough

This PR enforces publish-time validation for send_email steps (non-empty subject and non-empty lexical body) while allowing empty fields in drafts. It changes the default send_email subject to an empty string, adds editor logic to compute per-action errors and block activation, surfaces errors on canvas nodes and sidebar, adds conditional delete confirmation for email steps, refactors email-modal/preview validation, updates backend validation to reject activating automations with empty email subject/body, and adds/updates unit and e2e tests and fixtures.

Possibly related PRs

  • TryGhost/Ghost#28035: Modifies automations step-deletion flow, adding special confirmation/behavior for deleting send_email when email body is empty.
  • TryGhost/Ghost#27878: Related backend validation/schema work for automations send_email fields and JSON-parsed email_lexical.
  • TryGhost/Ghost#28117: Touches the same action-builder and email-content editor/placeholder logic used by this PR.

Suggested labels

ok to merge for me

Suggested reviewers

  • EvanHahn
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding warnings and errors for automation email steps, which aligns with the PR's core objective of improving validation and user feedback.
Description check ✅ Passed The description is well-related to the changeset, detailing specific changes (confirmation dialogs, save-vs-publish validation, deletion behavior) that align with the file modifications and PR objectives.
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 NY-1301_automations-errs

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 and usage tips.

Comment thread apps/posts/test/unit/views/automations/automation-editor.test.tsx Fixed
peterzimon and others added 12 commits June 4, 2026 15:34
ref https://linear.app/tryghost/issue/NY-1301/

Deleting an email step was immediate, which made accidental removals too easy while editing an automation draft. This adds a confirmation step for email deletion while preserving the existing draft-save flow.
ref https://linear.app/tryghost/issue/NY-1301/

Creating a new email step left the user on the canvas, so the next edit required an extra click and the browser could scroll the canvas while focusing the sidebar. This selects the inserted email immediately and focuses its subject field without scrolling.
ref https://linear.app/tryghost/issue/NY-1301/

Email steps can now start with an empty subject, so save and publish need clear validation feedback when a subject is required. This adds a toast plus inline card errors that only appear after validation and clear as users fix the affected email steps.
ref https://linear.app/tryghost/issue/NY-1301/

Opening an email step should put the user straight into the subject field, whether the email is newly created or selected later. This moves focus ownership into the email sidebar itself and keeps the existing no-scroll behavior.
ref https://linear.app/tryghost/issue/NY-1301/

This gives editors lightweight feedback when an automation email has no body while keeping blocking save errors visually prioritized.
ref https://linear.app/tryghost/issue/NY-1301/

This prevents editors from accidentally publishing automations that contain emails without body content while keeping empty bodies as warnings instead of blocking validation errors.
ref https://linear.app/tryghost/issue/NY-1301/

This replaces the disabled tail add-step affordance with a clearer non-interactive limit indicator and clarifies the email content editing action.
ref https://linear.app/tryghost/issue/NY-1301/

This avoids showing a wait-step validation error while the user is still editing the days field and waits until focus leaves the input.
ref https://linear.app/tryghost/issue/NY-1301/

This clarifies the save failure toast and delay input error so editors get more direct recovery guidance.
ref https://linear.app/tryghost/issue/NY-1301/

This tightens the maximum-step indicator copy and keeps the background stripe treatment balanced.
ref https://linear.app/tryghost/issue/NY-1301/

The rebase with the automation UX changes means right-click editing an existing email should seed the modal with the existing email body instead of an empty body.
@troyciesco troyciesco force-pushed the NY-1301_automations-errs branch from b5ac58b to 94a9a3f Compare June 4, 2026 19:34
@troyciesco troyciesco changed the title Ny 1301 automations errs Updated warnings and errors for automations email steps Jun 4, 2026
@troyciesco troyciesco marked this pull request as ready for review June 4, 2026 19:50
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 4, 2026

Codecov Report

❌ Patch coverage is 86.04651% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.79%. Comparing base (a035cf1) to head (fce88d1).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...ore/server/services/automations/automations-api.ts 86.04% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #28364      +/-   ##
==========================================
+ Coverage   73.77%   73.79%   +0.02%     
==========================================
  Files        1537     1536       -1     
  Lines      130707   130889     +182     
  Branches    15683    15688       +5     
==========================================
+ Hits        96425    96596     +171     
- Misses      33292    33303      +11     
  Partials      990      990              
Flag Coverage Δ
admin-tests 54.67% <ø> (ø)
e2e-tests 73.79% <86.04%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
apps/posts/src/views/Automations/components/automation-canvas.tsx (1)

345-362: ⚡ Quick win

Consider extracting isEmptyEmailLexical to a shared utility.

This function is duplicated in editor.tsx (lines 16-33) with identical logic. While not a critical issue, extracting it to a shared location would improve maintainability and reduce the risk of divergence.

🤖 Prompt for 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.

In `@apps/posts/src/views/Automations/components/automation-canvas.tsx` around
lines 345 - 362, Extract the duplicated isEmptyEmailLexical function into a
shared utility module (e.g., export a named function isEmptyEmailLexical from a
new util file) and replace both local implementations in automation-canvas.tsx
(function isEmptyEmailLexical) and editor.tsx with an import from that module;
keep the same function signature and behavior, export it as a named export,
update imports in both components, and remove the duplicate code blocks so both
files reuse the single shared implementation.
🤖 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 `@apps/posts/src/views/Automations/editor.tsx`:
- Around line 200-205: The onError handler in the editMutation.mutate call
should accept the mutation error and only show the validation-specific
description when there are actual validation errors; update the onError
signature to receive the error (e.g., onError: (err) => { ... }), call or
re-check validateActionErrors()/actionErrors (or check the existing actionErrors
state) and setEditState(errorState) as before, but only pass description: 'Fix
the highlighted steps and try again.' to toast.error when actionErrors is
non-empty (otherwise show a generic error message or no description), ensuring
this change is applied inside the editMutation.mutate onError handler referenced
in editor.tsx.

---

Nitpick comments:
In `@apps/posts/src/views/Automations/components/automation-canvas.tsx`:
- Around line 345-362: Extract the duplicated isEmptyEmailLexical function into
a shared utility module (e.g., export a named function isEmptyEmailLexical from
a new util file) and replace both local implementations in automation-canvas.tsx
(function isEmptyEmailLexical) and editor.tsx with an import from that module;
keep the same function signature and behavior, export it as a named export,
update imports in both components, and remove the duplicate code blocks so both
files reuse the single shared implementation.
🪄 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: 4d75c8d2-33ea-456f-a0eb-5fce9272b91d

📥 Commits

Reviewing files that changed from the base of the PR and between 24244bb and 864887e.

📒 Files selected for processing (10)
  • apps/admin-x-framework/src/api/automations.ts
  • apps/admin-x-framework/test/unit/api/automations.test.ts
  • apps/posts/src/views/Automations/components/automation-canvas.tsx
  • apps/posts/src/views/Automations/components/email-modal/email-content-modal.tsx
  • apps/posts/src/views/Automations/components/email-modal/use-email-preview.ts
  • apps/posts/src/views/Automations/editor.tsx
  • apps/posts/test/unit/views/automations/automation-editor.test.tsx
  • ghost/core/core/server/services/automations/automations-api.ts
  • ghost/core/test/e2e-api/admin/automations.test.js
  • ghost/core/test/unit/server/services/automations/automations-api.test.js

Comment thread apps/posts/src/views/Automations/editor.tsx Outdated
@troyciesco troyciesco merged commit 270bd3f into main Jun 4, 2026
53 checks passed
@troyciesco troyciesco deleted the NY-1301_automations-errs branch June 4, 2026 22:07
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.

3 participants