🐛 Fixed members being subscribed to all newsletters when choosing none - #30708
Conversation
no ref Unticking every newsletter on Portal's selection page sends `newsletters: []`, and theme signup forms with only unchecked newsletter checkboxes send the same to opt out of the defaults. Since `_validateNewsletters` was introduced, an empty list came back as `undefined`, which member creation reads as "not specified" and replaces with every subscribe-on-signup newsletter. Paid signups got the same treatment once checkout metadata went through that function too. An explicit empty list is now kept, so the member signs up to nothing. Only a missing list falls back to the defaults, which meant dropping the `?? []` at the signup call site that turned a missing list into an empty one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: QUIET Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (9)
🧰 Additional context used📓 Path-based instructions (5)Review new or changed service boundaries for explicit dependency ownership, deterministic/idempotent initialisation, boot ordering, transaction and event semantics, cache coherence, and restart/multi-instance safety.⚙️ CodeRabbit configuration file Files:
Review whether tests prove changed behaviour, meaningful error/edge paths, and externally observable contracts without coupling to implementation details.⚙️ CodeRabbit configuration file Files:
New source files must be TypeScript: flag new JS files as a required change unless exempt (DB migrations, apps/ember-admin/, tool/config files, scripts/, docker/, generated code).⚙️ CodeRabbit configuration file Files:
Prioritise concrete correctness, security, data-integrity, compatibility, and regression risks.⚙️ CodeRabbit configuration file Files:
New files are TypeScript: Fail if the PR adds a new .js/.jsx/.cjs/.mjs source file, unless it is: a DB migration (ghost/core/core/server/data/migrations/), under apps/ember-admin/, a tool/config file, under scripts/ or docker/, or generated...📄 CodeRabbit inference engine (Custom checks) Files:
🔇 Additional comments (2)
WalkthroughThe signup flow passes the newsletters field without applying an empty-array default. Newsletter validation returns Suggested reviewers: Priority: ⚪ Not assessed Change: Bug fix Merge Risk: ⚪ Minimal · up to The signup flow now preserves explicit opt-outs while retaining defaults for omitted newsletters, with matching test coverage. 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ast-grep (0.45.3)ghost/core/test/unit/server/services/members/members-api/controllers/router-controller.test.jsast-grep timed out on this file Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost:test:ci:integration |
✅ Succeeded | 4m 45s | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 3m 47s | View ↗ |
nx run ghost:test:ci:e2e |
✅ Succeeded | 4m 16s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 3m 9s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 54s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 29s | View ↗ |
nx run-many -t test:unit -p ghost |
✅ Succeeded | 35s | View ↗ |
nx run-many -t lint -p ghost,ghost-monorepo |
✅ Succeeded | 20s | View ↗ |
Additional runs (4) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-09-11 23:30:41 UTC
|
CI failure here ( I confirmed this reproduces on On the actual code: Generated by Claude Code |
no ref
A truthy, non-array value with a length of 0 (for example {length: 0}
in a hand-crafted request body) satisfied the new empty-list check and
was returned as [], the same as a real empty array, instead of falling
through to the defaults like any other malformed value. Requiring
Array.isArray keeps that check limited to actual arrays.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A2bVf68ihCcTAfhkWGBkAr
|
Addressed CodeRabbit's "Type-Safe Boundaries" finding: Generated by Claude Code |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #30708 +/- ##
==========================================
+ Coverage 67.66% 67.73% +0.07%
==========================================
Files 1676 1676
Lines 60517 60519 +2
Branches 10465 10466 +1
==========================================
+ Hits 40947 40991 +44
+ Misses 17250 17212 -38
+ Partials 2320 2316 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
A member reported that on a free plan, unticking every newsletter in Portal still subscribed them to all of them.
Portal's newsletter selection page sends
newsletters: []when nothing is ticked. Theme signup forms whose only newsletter inputs are uncheckeddata-members-newslettercheckboxes also send[], deliberately, to opt out of the defaults (apps/portal/src/data-attributes.js). The server turned that intoundefined, which member creation treats as "not specified" and fills with everysubscribe_on_signupnewsletter.This regressed twice:
_validateNewslettersreplaced passing the list through unchanged and returnedundefinedfor an empty array."[]"becameundefined, was dropped from the metadata, and the Stripe webhook applied the defaults.Change
_validateNewslettersnow returns[]for an empty list, andundefinedonly when no list was sent. The signup call site no longer turns a missing list into[](?? []), so signups that don't send newsletters (Portal's signup and offer pages, the signup-form embed) still get the defaults.Testing
Unit tests for a free signup sending
[], a signup sending no list (still gets the defaults), and paid checkout metadata keeping"[]".🤖 Generated with Claude Code