Skip to content

Added members_single_opt_in computed setting for single opt-in signup#26801

Closed
mike182uk wants to merge 1 commit intomainfrom
single-opt-in-config
Closed

Added members_single_opt_in computed setting for single opt-in signup#26801
mike182uk wants to merge 1 commit intomainfrom
single-opt-in-config

Conversation

@mike182uk
Copy link
Copy Markdown
Member

no ref

This is the first step toward native single opt-in support. The setting is derived from the hostSettings:membersSingleOptIn:enabled config value (managed by host) and exposed to frontends via the Content API public settings response.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 12, 2026

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: 7a119215-7f43-4fcc-a1a1-4bf47d0998a7

📥 Commits

Reviewing files that changed from the base of the PR and between 2119e61 and dc327c0.

⛔ Files ignored due to path filters (2)
  • ghost/core/test/e2e-api/admin/__snapshots__/settings.test.js.snap is excluded by !**/*.snap
  • ghost/core/test/e2e-api/content/__snapshots__/settings.test.js.snap is excluded by !**/*.snap
📒 Files selected for processing (5)
  • ghost/core/core/server/services/settings-helpers/settings-helpers.js
  • ghost/core/core/server/services/settings/settings-service.js
  • ghost/core/core/shared/settings-cache/public.js
  • ghost/core/test/e2e-api/admin/settings.test.js
  • ghost/core/test/unit/server/services/settings-helpers/settings-helpers.test.js
🚧 Files skipped from review as they are similar to previous changes (4)
  • ghost/core/core/server/services/settings/settings-service.js
  • ghost/core/core/server/services/settings-helpers/settings-helpers.js
  • ghost/core/test/e2e-api/admin/settings.test.js
  • ghost/core/test/unit/server/services/settings-helpers/settings-helpers.test.js

Walkthrough

Adds a new boolean setting members_single_opt_in exposed via the settings system. Implements SettingsHelpers.isMembersSingleOptInEnabled() to coerce the value from host config, adds a calculated settings field members_single_opt_in (derived at boot, no DB dependency), includes the key in the public settings cache allowlist, adds unit tests covering true/false/missing/no-config cases, and updates an E2E test expected settings count from 99 to 100.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding a new computed setting for single opt-in signup functionality.
Description check ✅ Passed The description is related to the changeset, explaining the purpose and context of the new setting being derived from host config and exposed via Content API.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch single-opt-in-config
📝 Coding Plan for PR comments
  • Generate coding plan

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

@mike182uk mike182uk force-pushed the single-opt-in-config branch from 51df4fc to 2119e61 Compare March 12, 2026 15:46
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.

🧹 Nitpick comments (1)
ghost/core/test/e2e-api/admin/settings.test.js (1)

14-14: Assert members_single_opt_in directly instead of only bumping the count.

matchSettingsArray() mostly validates length, so this still passes if the new key disappears and some other setting keeps the total at 100. Please add a focused assertion for members_single_opt_in, ideally on the public/Content API response this PR is targeting.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ghost/core/test/e2e-api/admin/settings.test.js` at line 14, The test relies
only on CURRENT_SETTINGS_COUNT and matchSettingsArray(), which primarily checks
length, so add a focused assertion to explicitly verify the presence and value
of the members_single_opt_in setting (instead of relying on count). Update the
test in settings.test.js to fetch the public/Content API response used in this
PR and assert that response.settings (or the settings array/object returned by
that API) contains a key members_single_opt_in with the expected boolean/string
value; keep the existing matchSettingsArray/CURRENT_SETTINGS_COUNT check but add
this explicit assertion to prevent silent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@ghost/core/test/e2e-api/admin/settings.test.js`:
- Line 14: The test relies only on CURRENT_SETTINGS_COUNT and
matchSettingsArray(), which primarily checks length, so add a focused assertion
to explicitly verify the presence and value of the members_single_opt_in setting
(instead of relying on count). Update the test in settings.test.js to fetch the
public/Content API response used in this PR and assert that response.settings
(or the settings array/object returned by that API) contains a key
members_single_opt_in with the expected boolean/string value; keep the existing
matchSettingsArray/CURRENT_SETTINGS_COUNT check but add this explicit assertion
to prevent silent regressions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 644304fb-36eb-4026-b599-747c6c1009e3

📥 Commits

Reviewing files that changed from the base of the PR and between 51df4fc and 2119e61.

⛔ Files ignored due to path filters (1)
  • ghost/core/test/e2e-api/admin/__snapshots__/settings.test.js.snap is excluded by !**/*.snap
📒 Files selected for processing (5)
  • ghost/core/core/server/services/settings-helpers/settings-helpers.js
  • ghost/core/core/server/services/settings/settings-service.js
  • ghost/core/core/shared/settings-cache/public.js
  • ghost/core/test/e2e-api/admin/settings.test.js
  • ghost/core/test/unit/server/services/settings-helpers/settings-helpers.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • ghost/core/core/server/services/settings/settings-service.js

no ref

This is the first step toward native single opt-in support. The setting
is derived from the `hostSettings:membersSingleOptIn:enabled` config
value (managed by host) and exposed to frontends via the Content
API public settings response.
@mike182uk mike182uk force-pushed the single-opt-in-config branch from 2119e61 to dc327c0 Compare March 12, 2026 16:16
@mike182uk
Copy link
Copy Markdown
Member Author

The work for this change is currently postponed

@mike182uk mike182uk closed this Mar 19, 2026
@mike182uk mike182uk deleted the single-opt-in-config branch March 26, 2026 16:38
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