✨ Added manual welcome email trigger for invite-only sites#28344
Open
deRonbrown wants to merge 1 commit into
Open
✨ Added manual welcome email trigger for invite-only sites#28344deRonbrown wants to merge 1 commit into
deRonbrown wants to merge 1 commit into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (16)
🚧 Files skipped from review as they are similar to previous changes (11)
WalkthroughAdds end-to-end welcome email support for members: admin UI checkbox and modal, client model/adapter/serializer changes, new POST endpoints and admin route, MemberBREADService logic to determine eligibility/automation presence and trigger welcome-email automations, output serialization of welcome_email_sent_at, and unit and e2e tests covering creation and explicit send flows. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ghost/core/core/server/api/endpoints/utils/serializers/output/members.js (1)
8-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRegister a serializer for
sendWelcomeEmail.
ghost/core/core/server/api/endpoints/members.jsnow exposescontroller.sendWelcomeEmail, but this module still doesn't export a matching serializer. Without that, the new endpoint won't return the standard{members: [...]}payload shape the admin member flows already use.Suggested fix
module.exports = { browse: createSerializer('browse', paginatedMembers), read: createSerializer('read', singleMember), edit: createSerializer('edit', singleMember), add: createSerializer('add', singleMember), + sendWelcomeEmail: createSerializer('sendWelcomeEmail', singleMember), destroy: createSerializer('destroy', passthrough),🤖 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 `@ghost/core/core/server/api/endpoints/utils/serializers/output/members.js` around lines 8 - 25, The module is missing a serializer entry for the new controller action sendWelcomeEmail; add a property named sendWelcomeEmail that uses createSerializer with the action name 'sendWelcomeEmail' and the singleMember serializer (i.e., add sendWelcomeEmail: createSerializer('sendWelcomeEmail', singleMember)) so the endpoint returns the standard {members: [...]} payload shape.
🧹 Nitpick comments (2)
ghost/core/test/unit/server/services/members/members-api/services/members-bread-service.test.js (1)
39-39: 💤 Low valueConsider testing
nullexplicitly.While
undefinedand empty string are tested,nullis a distinct value in JavaScript and could behave differently. Adding an explicit assertion fornullwould make the test coverage more complete.📝 Suggested addition
it('returns null for unknown statuses', function () { assert.equal(service.pickWelcomeEmailStatus('unknown'), null); assert.equal(service.pickWelcomeEmailStatus(''), null); assert.equal(service.pickWelcomeEmailStatus(undefined), null); + assert.equal(service.pickWelcomeEmailStatus(null), null); });🤖 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 `@ghost/core/test/unit/server/services/members/members-api/services/members-bread-service.test.js` at line 39, Add an explicit assertion for null to the existing test for pickWelcomeEmailStatus: call service.pickWelcomeEmailStatus(null) and assert it returns null (e.g., assert.equal(service.pickWelcomeEmailStatus(null), null)); this complements the existing checks for undefined and empty string and ensures null is handled as expected in the pickWelcomeEmailStatus logic.ghost/admin/app/components/members/modals/send-welcome-email.js (1)
28-32: ⚡ Quick winClosing the modal on error hides the GhTaskButton failure/retry state.
On failure you call
this.args.close(false)and then re-throw. Re-throwing normally letsGhTaskButtonsurface an error/retry state, but the button is destroyed once the modal closes, so that rethrow has no visible effect and the user must reopen the modal to retry. Consider not closing on error (let the button show the failure and allow an in-place retry); theshowAPIErrornotification already informs the user.♻️ Proposed change
} catch (e) { this.notifications.showAPIError(e, {key: 'member.send-welcome-email'}); - this.args.close(false); throw e; }🤖 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 `@ghost/admin/app/components/members/modals/send-welcome-email.js` around lines 28 - 32, The catch block in the send-welcome-email flow closes the modal (this.args.close(false)) before re-throwing, which destroys the GhTaskButton and prevents its error/retry state from being shown; remove the this.args.close(false) call from the catch so the modal stays open, call this.notifications.showAPIError(e, {key: 'member.send-welcome-email'}) as-is, and re-throw the error so GhTaskButton can display failure and allow in-place retry.
🤖 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/services/members/members-api/services/member-bread-service.js`:
- Around line 89-107: Scope the query to only welcome-email deliveries: update
the fetchWelcomeEmailSentAt method (and the other helper that also reads latest
AutomatedEmailRecipient) to add a filter that limits results to the
welcome-email automation before ordering by created_at. In the
AutomatedEmailRecipient query (the call chain on
this.AutomatedEmailRecipient.forge().query(...).fetch(...)), join or filter by
the related automated email/automation identifier (e.g., automated_emails.slug
or automation_type) that corresponds to the welcome-email (commonly "welcome" or
"welcome-email") and only then orderBy('created_at').limit(1); apply the same
change to the second helper that queries AutomatedEmailRecipient so only
welcome-email deliveries are considered.
- Around line 583-601: The current code uses model.get('status') (the model
returned from memberRepository.create) when calling pickWelcomeEmailStatus, but
subsequent calls like linkStripeCustomer and setComplimentarySubscription can
mutate the persisted member (to 'paid' or 'comped'), so re-read the persisted
status before deciding which automation to trigger: after the post-create
mutation helpers (linkStripeCustomer / setComplimentarySubscription) either
fetch the updated member from the repository (e.g. this.memberRepository.read or
similar) and call pickWelcomeEmailStatus on that fresh record, or change those
helper functions to return the updated model and use that returned
model.get('status') when invoking pickWelcomeEmailStatus and
triggerMemberSignupAutomation.
In `@ghost/core/test/e2e-api/admin/members.test.js`:
- Around line 3647-3651: The test for the POST /members/:id/welcome_email path
uses agent.post(`/members/${memberId}/welcome_email`) and
.matchBodySnapshot(...) but doesn't assert the HTTP status; add an explicit
status assertion (use .expectStatus(429)) to the agent chain for this request to
ensure the host-limit error returns the expected 429 status before calling
.matchBodySnapshot.
---
Outside diff comments:
In `@ghost/core/core/server/api/endpoints/utils/serializers/output/members.js`:
- Around line 8-25: The module is missing a serializer entry for the new
controller action sendWelcomeEmail; add a property named sendWelcomeEmail that
uses createSerializer with the action name 'sendWelcomeEmail' and the
singleMember serializer (i.e., add sendWelcomeEmail:
createSerializer('sendWelcomeEmail', singleMember)) so the endpoint returns the
standard {members: [...]} payload shape.
---
Nitpick comments:
In `@ghost/admin/app/components/members/modals/send-welcome-email.js`:
- Around line 28-32: The catch block in the send-welcome-email flow closes the
modal (this.args.close(false)) before re-throwing, which destroys the
GhTaskButton and prevents its error/retry state from being shown; remove the
this.args.close(false) call from the catch so the modal stays open, call
this.notifications.showAPIError(e, {key: 'member.send-welcome-email'}) as-is,
and re-throw the error so GhTaskButton can display failure and allow in-place
retry.
In
`@ghost/core/test/unit/server/services/members/members-api/services/members-bread-service.test.js`:
- Line 39: Add an explicit assertion for null to the existing test for
pickWelcomeEmailStatus: call service.pickWelcomeEmailStatus(null) and assert it
returns null (e.g., assert.equal(service.pickWelcomeEmailStatus(null), null));
this complements the existing checks for undefined and empty string and ensures
null is handled as expected in the pickWelcomeEmailStatus logic.
🪄 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: 4fc385a6-4030-4552-9c60-423a67bae8a0
⛔ Files ignored due to path filters (5)
ghost/core/test/e2e-api/admin/__snapshots__/member-commenting.test.js.snapis excluded by!**/*.snapghost/core/test/e2e-api/admin/__snapshots__/members-edit-subscriptions.test.js.snapis excluded by!**/*.snapghost/core/test/e2e-api/admin/__snapshots__/members-newsletters.test.js.snapis excluded by!**/*.snapghost/core/test/e2e-api/admin/__snapshots__/members.test.js.snapis excluded by!**/*.snapghost/core/test/e2e-api/members/__snapshots__/webhooks.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (16)
ghost/admin/app/adapters/member.jsghost/admin/app/components/gh-member-settings-form.hbsghost/admin/app/components/members/modals/send-welcome-email.hbsghost/admin/app/components/members/modals/send-welcome-email.jsghost/admin/app/controllers/member.jsghost/admin/app/models/member.jsghost/admin/app/routes/member/new.jsghost/admin/app/serializers/member.jsghost/admin/app/templates/member.hbsghost/core/core/server/api/endpoints/members.jsghost/core/core/server/api/endpoints/utils/serializers/output/members.jsghost/core/core/server/services/members/members-api/members-api.jsghost/core/core/server/services/members/members-api/services/member-bread-service.jsghost/core/core/server/web/api/endpoints/admin/routes.jsghost/core/test/e2e-api/admin/members.test.jsghost/core/test/unit/server/services/members/members-api/services/members-bread-service.test.js
Comment on lines
+89
to
+107
| async fetchWelcomeEmailSentAt(memberId) { | ||
| if (!this.AutomatedEmailRecipient || !memberId) { | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| const latest = await this.AutomatedEmailRecipient.forge() | ||
| .query((qb) => { | ||
| qb.where('member_id', memberId) | ||
| .orderBy('created_at', 'desc') | ||
| .limit(1); | ||
| }) | ||
| .fetch({require: false}); | ||
| return latest ? latest.get('created_at') : null; | ||
| } catch (err) { | ||
| logging.error(`Failed to load welcome email sent timestamp for member ${memberId}.`); | ||
| logging.error(err); | ||
| return null; | ||
| } |
Contributor
There was a problem hiding this comment.
Scope welcome_email_sent_at to welcome-email deliveries only.
Both helpers currently take the newest automated_email_recipients row for the member without filtering by automation/email type. Any later automated email can overwrite this field, so the UI will report a welcome email as already sent when only some other automated email was delivered. Filter these queries to the welcome-email automations before taking the latest timestamp.
Also applies to: 117-141
🤖 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
`@ghost/core/core/server/services/members/members-api/services/member-bread-service.js`
around lines 89 - 107, Scope the query to only welcome-email deliveries: update
the fetchWelcomeEmailSentAt method (and the other helper that also reads latest
AutomatedEmailRecipient) to add a filter that limits results to the
welcome-email automation before ordering by created_at. In the
AutomatedEmailRecipient query (the call chain on
this.AutomatedEmailRecipient.forge().query(...).fetch(...)), join or filter by
the related automated email/automation identifier (e.g., automated_emails.slug
or automation_type) that corresponds to the welcome-email (commonly "welcome" or
"welcome-email") and only then orderBy('created_at').limit(1); apply the same
change to the second helper that queries AutomatedEmailRecipient so only
welcome-email deliveries are considered.
In 6.18.0 welcome emails were wired into the self-signup flow only — admin-created members are intentionally skipped (the member repository gates the auto-enqueue on source='member'). Invite-only sites had no way to send the welcome email to manually-added members. This adds two opt-in paths that reuse the existing welcome email automation: - A "Send welcome email" toggle on /ghost/#/members/new, exposed via a new send_welcome_email option on POST /members. - A "Send/Resend welcome email" button on the member detail page, exposed via POST /members/:id/welcome_email. The button label and confirmation modal copy adapt to a new welcome_email_sent_at field surfaced from automated_email_recipients — the same source the existing activity feed reads, so no new event model is needed. Both paths enqueue a WelcomeEmailAutomationRun so the existing poll handles delivery and activity tracking. Both are short-circuited by the email_verification_required host limit. The resend endpoint returns 400 for ineligible statuses (comped) and 404 when no automation is configured.
deRonbrown
force-pushed
the
feature/manual-welcome-email
branch
from
def5293 to
94feefd
Compare
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
1 participant
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.
Why
In Ghost 6.18.0, welcome emails were wired into the self-signup flow only —
memberRepository.creategates the auto-trigger onsource='member'. Sites running in invite-only access mode (where staff manually add members) had no way to send the welcome email, even though the automation itself was set up and active.What
Two opt-in paths that reuse the existing welcome email automation:
/ghost/#/members/new— defaults off, exposed via a newsend_welcome_emailoption onPOST /members.POST /members/:id/welcome_email. The label, button text, and confirmation modal copy adapt to a newwelcome_email_sent_atfield, surfaced from the existingautomated_email_recipientstable (the same source the activity feed reads). No new event model.Both paths call
memberRepository.triggerMemberSignupAutomationso the existing poller handles delivery and activity tracking. Both are short-circuited by theemail_verification_requiredhost limit. The resend endpoint returns 400 for ineligible statuses (comped) and 404 when no automation is configured.Screenshots
Send welcome email toggle on the new member form:
Resend welcome email action on the member detail page:
Tests
welcome_email_sent_at: null)Verification
Checklist