Skip to content

Removed "member not found" option from welcome email automation runs#27457

Merged
troyciesco merged 2 commits intomainfrom
remove-unused-automation-run-exit-reason
Apr 22, 2026
Merged

Removed "member not found" option from welcome email automation runs#27457
troyciesco merged 2 commits intomainfrom
remove-unused-automation-run-exit-reason

Conversation

@EvanHahn
Copy link
Copy Markdown
Contributor

@EvanHahn EvanHahn commented Apr 20, 2026

closes https://linear.app/ghost/issue/NY-1192
ref #27456

Turns out that this value will never be set because of cascade deletions. Let's drop the option.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 20, 2026

Walkthrough

This pull request removes 'member not found' as a valid exit reason value from the welcome email automation runs feature. The change updates the schema validation constraints for the exit_reason column in the welcome_email_automation_runs table to exclude this value, retaining only 'email send failed', 'member unsubscribed', 'member changed status', and 'finished'. Correspondingly, the JSDoc type signature for the markExited() function in the poll service is updated to reflect the narrowed set of allowed exit reason values, maintaining consistency between the schema and service layer documentation.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and clearly describes the main change: removing 'member not found' from welcome email automation run exit reasons.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description check ✅ Passed The pull request description directly relates to the changeset by explaining the reason for removing 'member not found' from the exit_reason validation enum and JSDoc signature.

✏️ 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 remove-unused-automation-run-exit-reason

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.

@github-actions github-actions Bot added the migration [pull request] Includes migration for review label Apr 20, 2026
@github-actions

This comment was marked as outdated.

@EvanHahn EvanHahn force-pushed the remove-unused-automation-run-exit-reason branch from ec722ee to 9ae35e1 Compare April 20, 2026 12:44
Base automatically changed from NY-1192 to main April 20, 2026 20:27
@EvanHahn EvanHahn force-pushed the remove-unused-automation-run-exit-reason branch from 9ae35e1 to 50c6d46 Compare April 21, 2026 20:04
ref https://linear.app/ghost/issue/NY-1192

Turns out that this value will never be set because of cascade
deletions. Let's drop the option.
@EvanHahn EvanHahn force-pushed the remove-unused-automation-run-exit-reason branch from 50c6d46 to 73fc594 Compare April 21, 2026 20:05
@EvanHahn EvanHahn marked this pull request as ready for review April 21, 2026 20:05
@EvanHahn EvanHahn removed the migration [pull request] Includes migration for review label Apr 21, 2026
ready_at: {type: 'dateTime', nullable: true},
step_started_at: {type: 'dateTime', nullable: true},
step_attempts: {type: 'integer', unsigned: true, nullable: false, defaultTo: 0},
exit_reason: {type: 'string', maxlength: 50, nullable: true, validations: {isIn: [['member not found', 'email send failed', 'member unsubscribed', 'member changed status', 'finished']]}},
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Theoretically, we should create a database migration to drop these rows.

Practically, no such rows should exit. We never had code to create them, and any relevant code is development-only right now.

(The original migration mentioned this value and I don't think we need to drop it. In other words, I don't think these validations got encoded into the schema.)

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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ghost/core/core/server/data/schema/schema.js`:
- Line 1202: Update the 6.29 migration that sets the newsletters table/column
validation for exit_reason so its validations.isIn array matches the runtime
schema: remove the "member not found" entry from the exit_reason allowed values
(the same field referenced as exit_reason with validations.isIn), ensuring the
migration's enum/list no longer contains that value and preserving the other
values ("email send failed", "member unsubscribed", "member changed status",
"finished").
🪄 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: 81abc2e2-8619-4f33-b5cc-271619d418f6

📥 Commits

Reviewing files that changed from the base of the PR and between 3a030fd and 73fc594.

📒 Files selected for processing (2)
  • ghost/core/core/server/data/schema/schema.js
  • ghost/core/core/server/services/welcome-email-automations/poll.js

Comment thread ghost/core/core/server/data/schema/schema.js
@EvanHahn EvanHahn requested a review from troyciesco April 22, 2026 19:36
@sonarqubecloud
Copy link
Copy Markdown

@troyciesco troyciesco merged commit e76c7ce into main Apr 22, 2026
43 checks passed
@troyciesco troyciesco deleted the remove-unused-automation-run-exit-reason branch April 22, 2026 20:31
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.

2 participants