Passed whole newsletter object on signup #22389
Conversation
WalkthroughThe pull request consists of two primary updates. First, in the 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
apps/portal/src/components/pages/NewsletterSelectionPage.jsOops! Something went wrong! :( ESLint: 8.44.0 ESLint couldn't find the plugin "eslint-plugin-i18next". (The package "eslint-plugin-i18next" was not found when loaded as a Node module from the directory "/apps/portal".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-i18next" was referenced from the config file in "apps/portal/package.json". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/portal/src/components/pages/NewsletterSelectionPage.js(2 hunks)ghost/i18n/locales/context.json(1 hunks)
🔇 Additional comments (2)
ghost/i18n/locales/context.json (1)
144-144: New localization string addedA new localization entry has been added for restricted email domains. This string will be shown to users attempting to sign up with email domains that are restricted from creating memberships.
apps/portal/src/components/pages/NewsletterSelectionPage.js (1)
109-109:✅ Verification successful
Changed to pass full newsletter objects instead of just IDs
The implementation now passes the entire newsletter objects rather than extracting just the IDs. This aligns with the PR title "Passed whole newsletter object on signup".
Let's verify that the backend API can handle receiving the full newsletter objects instead of just IDs:
🏁 Script executed:
#!/bin/bash # Look for API endpoints that handle the signup process to confirm they expect newsletter objects # Search for API routes or handlers related to signup that process newsletters rg -A 10 "signup|member.*create|newsletter.*subscribe" --glob "*.js" | grep -v "test" # Search for functions that consume the newsletter data from the payload ast-grep --pattern $'function $fn($payload) { $$$ payload.$$$newsletters$$$ $$$ }'Length of output: 127151
Confirmation: Full newsletter objects are correctly handled by the API
The verification shows that the signup flow now forwards the complete newsletter objects. In the actions and API utility modules the
newslettersproperty is preserved and later converted via JSON (e.g. in metadata), which confirms that the backend is set up to consume the full objects rather than just IDs. No additional changes are necessary.
| @@ -48,6 +48,7 @@ function NewsletterPrefSection({newsletter, subscribedNewsletters, setSubscribed | |||
| } | |||
|
|
|||
| function NewsletterPrefs({subscribedNewsletters, setSubscribedNewsletters}) { | |||
| console.log('subscribedNewsletters', subscribedNewsletters); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove console.log statement before production
This debugging statement should be removed before pushing to production. Console logging can expose potentially sensitive information and adds unnecessary noise to browser consoles.
- console.log('subscribedNewsletters', subscribedNewsletters);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log('subscribedNewsletters', subscribedNewsletters); | |
| // (The console.log statement has been removed) |
No description provided.