Unified members/paid-members enabled checks in Admin #22721
Conversation
WalkthroughThe changes refactor the handling of member-related functionality throughout the Ghost admin interface. References to the Possibly related PRs
Suggested labels
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ghost/admin/app/components/gh-member-settings-form.jsOops! Something went wrong! :( ESLint: 8.57.1 Error: Failed to load parser '@babel/eslint-parser' declared in 'ghost/admin/.eslintrc.js': Cannot find module '@babel/eslint-parser'
ghost/admin/app/components/gh-members-recipient-select.jsOops! Something went wrong! :( ESLint: 8.57.1 Error: Failed to load parser '@babel/eslint-parser' declared in 'ghost/admin/.eslintrc.js': Cannot find module '@babel/eslint-parser'
ghost/admin/app/components/koenig-lexical-editor.jsOops! Something went wrong! :( ESLint: 8.57.1 Error: Failed to load parser '@babel/eslint-parser' declared in 'ghost/admin/.eslintrc.js': Cannot find module '@babel/eslint-parser'
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (30)
💤 Files with no reviewable changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (49)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
no issue - we had a confusing mix of manual underlying access setting checks, direct `settings.membersEnabled/paidMembersEnabled` checks, and `membersUtils.membersEnabled/paidMembersEnabled checks - the `membersUtils.xEnabled` checks were originally in place to remove the repeated direct checks of `settings.membersSignupAccess !== 'none'` type checks that were spread around the codebase but the refactor to switch to those utils was never fully completed - we've later added API-level `membersEnabled` and `paidMembersEnabled` settings which `membersUtils.xEnabled` checks were switched to but we also started using the underlying settings directly in different parts of the codebase meaning we ended up with three different approaches - the confusion around and slightly different behaviour between these different methods meant are tests were also difficult to determine as setup helpers were doing different things that meant code paths using each different approach were not necessarily doing what was intended - unified to always use `settings.membersEnabled` and `settings.paidMembersEnabled` - removed the duplicate properties from `membersUtils` - switched all uses of `membersUtils` and manual checks over to `settings` checks - updated test utils to update all impacted settings when enabling/disabling members - fixed tests that were incorrectly testing against members disabled - added missing `/api/admin/stats/mrr/` mocked endpoint that was causing dashboard tests to fail now that we are correctly testing against the members enabled state
kevinansfield
force-pushed
the
unify-admin-members-enabled
branch
from
561506d to
92b3fef
Compare
|
Cleaning up older PRs 💅 |
Pull request was closed
2 participants
no issue
settings.membersEnabled/paidMembersEnabledchecks, andmembersUtils.membersEnabled/paidMembersEnabledchecksmembersUtils.xEnabledchecks were originally in place to remove the repeated direct checks ofsettings.membersSignupAccess !== 'none'type checks that were spread around the codebase but the refactor to switch to those utils was never fully completedmembersEnabledandpaidMembersEnabledsettings whichmembersUtils.xEnabledchecks were switched to but we also started using the underlying settings directly in different parts of the codebase meaning we ended up with three different approachessettings.membersEnabledandsettings.paidMembersEnabledmembersUtilsmembersUtilsand manual checks over tosettingschecks/api/admin/stats/mrr/mocked endpoint that was causing dashboard tests to fail now that we are correctly testing against the members enabled state