🐛 Fixed email domain blocklist not being checked when a member updates their email address #22320
Conversation
WalkthroughThis pull request implements several updates related to email domain validation and error messaging across different modules. The Possibly related PRs
Suggested labels
Suggested reviewers
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/actions.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. ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
ronaldlangeveld
changed the title
There was a problem hiding this comment.
Hey @ronaldlangeveld, LGTM on general, left two comments :)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (55)
ghost/i18n/locales/ne/portal.json (1)
107-108: New Localization Entry Added (Nepali):
The key
"Memberships from this email domain are currently restricted."
has been added with an empty translation value. This is in line with our translation workflow where new keys are initially set to empty strings. Please ensure that translator comments or context are provided (if needed) so that the translation team understands the intended use of this message.ghost/i18n/locales/it/portal.json (1)
107-107: New Localization Entry Added (Italian):
The new key
"Memberships from this email domain are currently restricted."
has been introduced with an empty string as its translation. This maintains consistency with similar updates in other locales. Consider adding any necessary context for translators if it isn’t already documented elsewhere.ghost/i18n/locales/sl/portal.json (1)
107-107: New Localization Entry Added (Slovenian):
The key
"Memberships from this email domain are currently restricted."
now appears with an empty translation string. This mirrors the updates in other localization files. As a next step, please confirm that translators have the necessary context to provide an appropriate translation later.ghost/i18n/locales/es/portal.json (1)
107-108: New Localization Entry Added (Spanish):
The entry
"Memberships from this email domain are currently restricted."
has been added with an empty string as its value. This addition is consistent with the coordinated update across multiple locales. Verify that this key is documented in the translation guidelines so that translators can supply the correct Spanish term in due course.ghost/i18n/locales/pt-BR/portal.json (1)
107-108: New Localization Entry Added (Portuguese - Brazil):
The new key
"Memberships from this email domain are currently restricted."
is present with an empty translation value. This follows our established process (as noted in previous PR learnings) where new translation keys are added with an empty string pending later translation. Ensure that the context for this key—especially its relation to blocked email domains—is well documented for the translators.ghost/i18n/locales/en/portal.json (1)
107-107: New Localization Key Added (English):
The key
"Memberships from this email domain are currently restricted."
has been added with an empty value. Please ensure that the error handling code (e.g. in MemberController.js, actions.js, and errors.js) looks up this exact key. If a placeholder is needed for translators, consider using a note or a temporary value such as "TBD" so that it is not mistaken for a missing translation later.ghost/i18n/locales/eo/portal.json (1)
107-107: New Localization Entry for Membership Restriction (Esperanto)
A new key"Memberships from this email domain are currently restricted."has been added with an empty translation value. Please confirm that either a proper Esperanto translation is provided or that the application’s fallback logic is in place to display the English message.ghost/i18n/locales/bg/portal.json (1)
107-107: New Localization Entry Added (Bulgarian)
The new entry"Memberships from this email domain are currently restricted."appears with an empty translation. Verify that a suitable Bulgarian translation will be provided soon or that fallback handling is acceptable for now.ghost/i18n/locales/ja/portal.json (1)
107-107: New Localization Entry with Pending Japanese Translation
The new key"Memberships from this email domain are currently restricted."has been added, but its translation value is empty. Please update it with a proper Japanese translation to ensure consistency across locales and improve user experience.ghost/i18n/locales/ru/portal.json (1)
107-107: New Russian Localization Entry Missing Translation
The key"Memberships from this email domain are currently restricted."is now present with an empty string. Ensure that you add the correct Russian translation before release to maintain consistency with other locale files.ghost/i18n/locales/fi/portal.json (1)
107-107: New Finnish Localization Entry for Membership Restriction
The file now includes the new key"Memberships from this email domain are currently restricted."with an empty translation value. Please verify that the Finnish translation is completed or that an appropriate fallback mechanism is active.ghost/i18n/locales/mn/portal.json (1)
107-108: Localization Entry: New Membership Block Message AddedA new key
"Memberships from this email domain are currently restricted."has been added with an empty translation. Please ensure that a proper Mongolian translation is provided to maintain localization consistency.ghost/i18n/locales/hr/portal.json (1)
107-108: Localization Entry: New Blocked Domain Message in CroatianThe new entry
"Memberships from this email domain are currently restricted."has been added with an empty value. Verify if a proper Croatian translation should be provided here to ensure users receive clear feedback in their native language.ghost/i18n/locales/vi/portal.json (1)
107-108: Localization Entry: New Blocked Domain Message in VietnameseThe added key
"Memberships from this email domain are currently restricted."currently has an empty translation. Please confirm if a Vietnamese translation is available or planned so that the user experience remains consistent.ghost/i18n/locales/sr/portal.json (1)
107-108: Localization Entry: New Membership Block Message in SerbianA new key
"Memberships from this email domain are currently restricted."appears with an empty translation. Additionally, note that the subsequent key for unavailable memberships is also empty; please verify that these entries are updated with the appropriate Serbian translations.ghost/i18n/locales/uz/portal.json (1)
107-108: Localization Entry: New Restricted Domain Message in UzbekThe new message
"Memberships from this email domain are currently restricted."has been added with an empty translation. Please provide the proper Uzbek text for this message to maintain consistency across all supported languages.ghost/i18n/locales/ro/portal.json (1)
107-107: New Localization Entry Added (Romanian)
The new key
"Memberships from this email domain are currently restricted.": ""
has been added with an empty translation. Please confirm if an empty string is intentional for now or if a proper Romanian translation will be provided in a follow-up update.ghost/i18n/locales/de/portal.json (1)
107-107: New Localization Entry Added (German)
The entry
"Memberships from this email domain are currently restricted.": ""
is now present with an empty value. Ensure that this placeholder is acceptable for German users or add a proper translation when possible for consistency in localization.ghost/i18n/locales/kz/portal.json (1)
107-107: New Localization Entry Added (Kazakh)
A new message
"Memberships from this email domain are currently restricted.": ""
has been inserted with an empty string. Please verify if this is the intended behavior for Kazakh or if a localized translation will be added in a later update.ghost/i18n/locales/nl/portal.json (1)
107-107: New Localization Entry Added (Dutch)
The key
"Memberships from this email domain are currently restricted.": ""
has been added with an empty translation. Confirm that the empty value is acceptable for Dutch or update it with the appropriate translation before launch.ghost/i18n/locales/zh/portal.json (1)
107-107: New Localization Entry Added (Chinese)
A new localization entry for the error message has been introduced:
"Memberships from this email domain are currently restricted.": ""
It currently remains empty. Please verify if this placeholder is intentional and note if a proper Chinese translation will be added later to maintain consistent user experience.ghost/i18n/locales/af/portal.json (1)
107-107: New localization string added for restricted email domains.The addition of this new localization string supports the new feature for blocking email domains during profile updates. The empty string indicates no Afrikaans translation is available yet.
Consider providing a translation for this string to ensure a consistent user experience for Afrikaans users. Alternatively, you could create a follow-up task for translations of this new string across all locales.
ghost/i18n/locales/is/portal.json (1)
107-107: New localization string added for restricted email domains.The addition of this string aligns with the PR's objective to implement domain blocking functionality for member email updates.
Consider providing an Icelandic translation or creating a follow-up task to ensure complete localization coverage across all languages.
ghost/i18n/locales/si/portal.json (1)
107-107: New localization string added for restricted email domains.This addition is consistent with the implementation of the blocked domain feature across the application.
As with other locales, consider adding a Sinhala translation for this string to provide a fully localized experience for users.
ghost/i18n/locales/da/portal.json (1)
107-107: New localization string added for restricted email domains.This addition ensures Danish users will see the appropriate message when attempting to update their email to a blocked domain.
Consider providing a Danish translation for this string to enhance the user experience for Danish-speaking users. Danish translations are well-maintained in this file, so keeping consistency would be beneficial.
ghost/i18n/locales/hu/portal.json (1)
107-107: Empty translation for membership restriction message in Hungarian.
The new key"Memberships from this email domain are currently restricted."has an empty value. Please confirm if this is intentional (to fallback to the default language) or if a proper Hungarian translation should be provided.ghost/i18n/locales/sw/portal.json (2)
107-108: Missing Swahili translation for membership restrictions.
The key"Memberships from this email domain are currently restricted."is added with an empty value. Please consider adding an appropriate Swahili translation or a meaningful placeholder if you intend to fallback on another language.
143-143: Missing Swahili translation for signups restriction message.
The new key"Signups from this email domain are currently restricted."is also introduced with an empty string. Verify whether this is meant to fall back to English or update it with a proper Swahili translation.ghost/i18n/locales/fa/portal.json (1)
107-107: Persian translation not provided for membership error message.
The new key"Memberships from this email domain are currently restricted."has an empty string as its translation value. If a Persian translation is available, please add it; otherwise, ensure that the fallback behavior is acceptable for your users.ghost/i18n/locales/ur/portal.json (2)
107-107: Missing Urdu translation for membership restriction message.
The key"Memberships from this email domain are currently restricted."is introduced with an empty value. Please determine if an Urdu translation is needed or if the fallback to English is intended.
143-143: Missing Urdu translation for signups restriction message.
Similarly, the key"Signups from this email domain are currently restricted."appears with an empty translation. Consider adding the proper Urdu content to maintain consistency in user messaging.ghost/i18n/locales/lv/portal.json (1)
107-107: Latvian translation is missing for the membership restriction key.
The new entry"Memberships from this email domain are currently restricted."has an empty string value. Please check if you plan to provide a Latvian translation now or later and ensure this meets the intended fallback strategy.ghost/i18n/locales/he/portal.json (1)
107-107: New localization key added with an empty translation.
The key"Memberships from this email domain are currently restricted."has been introduced at line 107 with an empty value. Please confirm whether this is intentional (e.g. awaiting a proper Hebrew translation) or if a placeholder should be provided.ghost/i18n/locales/uk/portal.json (1)
107-107: New localization key added with an empty translation.
Similar to the Hebrew file, the key"Memberships from this email domain are currently restricted."is added at line 107 with an empty string as its translation. Please verify that this empty value is intended or update it with the appropriate Ukrainian translation if available.ghost/i18n/locales/sk/portal.json (1)
107-107: New localization key detected with missing translation.
At line 107, the key"Memberships from this email domain are currently restricted."is added with an empty translation. Ensure that this entry aligns with localization practices for Slovak and that a proper translation is provided either now or via a follow-up task.ghost/i18n/locales/nn/portal.json (1)
107-107: New localization key added with an empty value.
The new key"Memberships from this email domain are currently restricted."appears at line 107 with an empty translation. Please confirm whether the empty string is a placeholder for a future Norwegian translation, or if it should be updated immediately.ghost/i18n/locales/bs/portal.json (1)
107-107: New localization key added with an empty translation.
Line 107 introduces"Memberships from this email domain are currently restricted."with an empty string. This matches the pattern seen across other localization files. Verify that the empty translation is acceptable for Bosnian or schedule a follow-up to provide a proper translation.ghost/i18n/locales/de-CH/portal.json (1)
107-107: New Localization Entry for Restricted Email DomainsThe key
"Memberships from this email domain are currently restricted."has been added with an empty translation. Please confirm that an empty value is intentional (for example, if a proper translation is pending) and that the key matches exactly across all locales.ghost/i18n/locales/cs/portal.json (1)
107-107: New Localization Entry for Blocked Email Domain (Czech)A new key
"Memberships from this email domain are currently restricted."has been introduced with an empty string. Verify whether you plan to provide a Czech translation later or if it should remain empty as a placeholder.ghost/i18n/locales/fr/portal.json (1)
107-107: New Localization Entry for Restricted Email Domain (French)The message key
"Memberships from this email domain are currently restricted."now appears with an empty translation. Please consider providing a suitable French translation or add a TODO comment if the translation is forthcoming.ghost/i18n/locales/id/portal.json (1)
107-107: New Localization Entry for Restricted Email Domain (Indonesian)A new localization key
"Memberships from this email domain are currently restricted."has been added with an empty value. Ensure that this is intentional and that the Indonesian translation will be reviewed or updated in a future commit.ghost/i18n/locales/ta/portal.json (1)
107-107: New Localization Entry for Restricted Email Domain (Tamil)The key
"Memberships from this email domain are currently restricted."is added with an empty translation. Please verify if the empty string is meant to signal a pending translation and that the key remains consistent with other locale files.ghost/i18n/locales/th/portal.json (1)
107-107: New Localization Entry Placeholder for Thai:
The key"Memberships from this email domain are currently restricted."is newly added with an empty translation. Please confirm whether leaving the translation empty is intentional—for example, as a placeholder for later updates—or if a proper Thai translation should be provided now.ghost/i18n/locales/sv/portal.json (1)
107-107: New Localization Entry Placeholder for Swedish:
The entry"Memberships from this email domain are currently restricted."appears with an empty string as its translation. Please verify if this empty value is intentional as a temporary placeholder or if a Swedish translation should be supplied immediately.ghost/i18n/locales/ko/portal.json (1)
107-107: New Localization Entry Placeholder for Korean:
The new key"Memberships from this email domain are currently restricted."has been added with no translation provided (empty string). Confirm if this is meant to fall back to the default English message or if a proper Korean translation is expected at this stage.ghost/i18n/locales/ar/portal.json (1)
107-107: New Localization Entry Placeholder for Arabic:
A new localization entry"Memberships from this email domain are currently restricted."is present with an empty value. Please check if leaving this field empty is by design (to fall back to a default) or if an Arabic translation should be provided.ghost/i18n/locales/mk/portal.json (1)
107-107: New Localization Entry Placeholder for Macedonian:
The key"Memberships from this email domain are currently restricted."has been added with an empty string as its value. Verify whether this empty placeholder is acceptable for now or if a Macedonian translation is required immediately.ghost/i18n/locales/et/portal.json (1)
107-107: Add translation for the new error message.This new error message is correctly added to the localization file but is missing its Estonian translation. Consider adding the appropriate translation or flagging this for the translation team.
ghost/i18n/locales/bn/portal.json (1)
107-107: Add translation for the new error message.This new error message is correctly added to the localization file but is missing its Bengali translation. Consider adding the appropriate translation or flagging this for the translation team.
ghost/i18n/locales/pl/portal.json (1)
107-107: Add translation for the new error message.This new error message is correctly added to the localization file but is missing its Polish translation. Consider adding the appropriate translation or flagging this for the translation team.
ghost/i18n/locales/zh-Hant/portal.json (1)
107-107: Add translation for the new error message.This new error message is correctly added to the localization file but is missing its Traditional Chinese translation. Consider adding the appropriate translation or flagging this for the translation team.
ghost/core/test/e2e-api/members/send-magic-link.test.js (1)
388-388: Consider mocking the identity field appropriately.The test uses a hardcoded identity value (
'12345678'), which might not be realistic. According to a previous comment, "there's currently no member update email tests, since our E2E test suite doesn't have a way of populating/mocking the identity field." Consider documenting this limitation or implementing a more robust way to test this functionality.apps/portal/src/utils/api.js (1)
384-390: Enhanced error handling with specific error messagesThe update to use async/await to properly parse the JSON error response is a good improvement. This ensures that users receive the specific error message about blocked domains rather than a generic error message.
Consider adding a try/catch block around the
await res.json()call to handle cases where the response might not contain valid JSON:}).then(async function (res) { if (res.ok) { return 'Success'; } else { - const errData = await res.json(); - const errMssg = errData?.errors?.[0]?.message || 'Failed to send email address verification email'; + let errMssg = 'Failed to send email address verification email'; + try { + const errData = await res.json(); + if (errData?.errors?.[0]?.message) { + errMssg = errData.errors[0].message; + } + } catch (e) { + // Could not parse JSON, use default error message + } throw new Error(errMssg); } });ghost/members-api/lib/controllers/MemberController.js (1)
53-53: Consider adding email validation check.The code assumes the email format is valid. Consider adding basic validation to ensure the email contains an '@' symbol before attempting to split it.
- const emailDomain = req.body.email.split('@')[1]?.toLowerCase(); + const email = req.body.email || ''; + const isValidEmail = email.includes('@'); + const emailDomain = isValidEmail ? email.split('@')[1]?.toLowerCase() : null;apps/portal/src/actions.js (1)
486-487: Consider using error codes rather than string matching.Relying on exact error message strings is brittle and could break if the message text changes. Consider having the API return error codes that can be matched instead.
A better approach would be:
- if (emailUpdate.error.message === 'Memberships from this email domain are currently restricted.') { + if (emailUpdate.error.code === 'BLOCKED_EMAIL_DOMAIN') {This would require updating the error thrown in
MemberController.jsto include a code property.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ghost/core/test/e2e-api/members/__snapshots__/send-magic-link.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (67)
apps/portal/src/actions.js(2 hunks)apps/portal/src/utils/api.js(1 hunks)apps/portal/src/utils/errors.js(1 hunks)ghost/core/core/server/web/members/app.js(1 hunks)ghost/core/test/e2e-api/members/send-magic-link.test.js(1 hunks)ghost/i18n/locales/af/portal.json(1 hunks)ghost/i18n/locales/ar/portal.json(1 hunks)ghost/i18n/locales/bg/portal.json(1 hunks)ghost/i18n/locales/bn/portal.json(1 hunks)ghost/i18n/locales/bs/portal.json(1 hunks)ghost/i18n/locales/ca/portal.json(1 hunks)ghost/i18n/locales/cs/portal.json(1 hunks)ghost/i18n/locales/da/portal.json(1 hunks)ghost/i18n/locales/de-CH/portal.json(1 hunks)ghost/i18n/locales/de/portal.json(1 hunks)ghost/i18n/locales/el/portal.json(1 hunks)ghost/i18n/locales/en/portal.json(1 hunks)ghost/i18n/locales/eo/portal.json(1 hunks)ghost/i18n/locales/es/portal.json(1 hunks)ghost/i18n/locales/et/portal.json(1 hunks)ghost/i18n/locales/fa/portal.json(1 hunks)ghost/i18n/locales/fi/portal.json(1 hunks)ghost/i18n/locales/fr/portal.json(1 hunks)ghost/i18n/locales/gd/portal.json(1 hunks)ghost/i18n/locales/he/portal.json(1 hunks)ghost/i18n/locales/hi/portal.json(1 hunks)ghost/i18n/locales/hr/portal.json(1 hunks)ghost/i18n/locales/hu/portal.json(1 hunks)ghost/i18n/locales/id/portal.json(1 hunks)ghost/i18n/locales/is/portal.json(1 hunks)ghost/i18n/locales/it/portal.json(1 hunks)ghost/i18n/locales/ja/portal.json(1 hunks)ghost/i18n/locales/ko/portal.json(1 hunks)ghost/i18n/locales/kz/portal.json(1 hunks)ghost/i18n/locales/lt/portal.json(1 hunks)ghost/i18n/locales/lv/portal.json(1 hunks)ghost/i18n/locales/mk/portal.json(1 hunks)ghost/i18n/locales/mn/portal.json(1 hunks)ghost/i18n/locales/ms/portal.json(1 hunks)ghost/i18n/locales/ne/portal.json(1 hunks)ghost/i18n/locales/nl/portal.json(1 hunks)ghost/i18n/locales/nn/portal.json(1 hunks)ghost/i18n/locales/no/portal.json(1 hunks)ghost/i18n/locales/pl/portal.json(1 hunks)ghost/i18n/locales/pt-BR/portal.json(1 hunks)ghost/i18n/locales/pt/portal.json(1 hunks)ghost/i18n/locales/ro/portal.json(1 hunks)ghost/i18n/locales/ru/portal.json(1 hunks)ghost/i18n/locales/si/portal.json(1 hunks)ghost/i18n/locales/sk/portal.json(1 hunks)ghost/i18n/locales/sl/portal.json(1 hunks)ghost/i18n/locales/sq/portal.json(1 hunks)ghost/i18n/locales/sr-Cyrl/portal.json(1 hunks)ghost/i18n/locales/sr/portal.json(1 hunks)ghost/i18n/locales/sv/portal.json(1 hunks)ghost/i18n/locales/sw/portal.json(1 hunks)ghost/i18n/locales/ta/portal.json(1 hunks)ghost/i18n/locales/th/portal.json(1 hunks)ghost/i18n/locales/tr/portal.json(1 hunks)ghost/i18n/locales/uk/portal.json(1 hunks)ghost/i18n/locales/ur/portal.json(1 hunks)ghost/i18n/locales/uz/portal.json(1 hunks)ghost/i18n/locales/vi/portal.json(1 hunks)ghost/i18n/locales/zh-Hant/portal.json(1 hunks)ghost/i18n/locales/zh/portal.json(1 hunks)ghost/members-api/lib/controllers/MemberController.js(5 hunks)ghost/members-api/lib/members-api.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
ghost/i18n/locales/pt/portal.json (1)
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/pt-BR/portal.json:18-18
Timestamp: 2025-02-12T20:09:12.731Z
Learning: In the Ghost project, new translation keys are first added with empty string values, and the actual translations are added later in separate PRs to maintain better organization of translation work.
ghost/i18n/locales/pt-BR/portal.json (1)
Learnt from: cathysarisky
PR: TryGhost/Ghost#22175
File: ghost/i18n/locales/pt-BR/portal.json:18-18
Timestamp: 2025-02-12T20:09:12.731Z
Learning: In the Ghost project, new translation keys are first added with empty string values, and the actual translations are added later in separate PRs to maintain better organization of translation work.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Ghost-CLI tests
🔇 Additional comments (23)
ghost/i18n/locales/pt/portal.json (1)
107-107: Novo Entrada para Localização (Portuguese):
Foi adicionada a chave
"Memberships from this email domain are currently restricted."
com um valor vazio. Isto segue o padrão utilizado no projeto para novas chaves, permitindo que a tradução seja fornecida posteriormente. Apenas verifique se o identificador da chave corresponde exatamente àquele utilizado em outros módulos para mensagens de erro.ghost/i18n/locales/tr/portal.json (1)
107-107: Yeni Yerelleştirme Anahtarı Eklendi (Turkish):
Satır 107’de
"Memberships from this email domain are currently restricted."
anahtarının boş bir değerle eklendiğini görüyoruz. Lütfen bu anahtarın, e-posta güncelleme işlemi sırasında engellenmiş alan adı hatalarını bildirirken kullanılan diğer yerelleştirme anahtarlarıyla tutarlı olduğundan emin olun.ghost/i18n/locales/ms/portal.json (1)
107-107: Entri Baru untuk Lokalisasi (Malay):
Kunci
"Memberships from this email domain are currently restricted."
telah ditambah pada baris 107 dengan nilai kosong. Pastikan ia sama persis dengan kunci di bahasa lain dan bahwa penerjemah diberi panduan untuk melengkapkan terjemahan yang sesuai nantinya.ghost/i18n/locales/el/portal.json (1)
107-107: Νέα Εγγραφή Τοπικοποίησης Προστέθηκε (Greek):
Στη γραμμή 107 προστέθηκε το κλειδί
"Memberships from this email domain are currently restricted."
με κενή τιμή. Βεβαιωθείτε ότι το κλειδί αυτό είναι σύμφωνο με εκείνα που χρησιμοποιούνται στην εφαρμογή για την εμφάνιση σχετικών μηνυμάτων σφάλματος και ότι οι μεταφραστές θα ενημερωθούν να συμπληρώσουν την κατάλληλη μετάφραση σε μεταγενέστερο στάδιο.apps/portal/src/utils/errors.js (1)
64-64:✅ Verification successful
Added specialized error message for blocked email domains.
Good addition of a new specialized error message that aligns with the PR objective to implement domain-based email restrictions. This message will be properly handled by the error processing system and displayed to users in their selected language.
Let's ensure that this error message is being properly referenced in the member email update logic:
🏁 Script executed:
#!/bin/bash # Check where this error message is being used in the codebase rg "Memberships from this email domain are currently restricted" --type js --no-headingLength of output: 790
Reference Usage Confirmed for Blocked Email Domain Error Message
The error message is used consistently throughout the codebase, including in the email update logic (e.g., withinapps/portal/src/actions.js). The query confirms that the specialized error message is referenced correctly in all relevant locations:
apps/portal/src/actions.js(used in the email update handling logic)ghost/members-api/lib/controllers/MemberController.js(for blocked email domain handling)ghost/core/test/e2e-api/members/send-magic-link.test.js(in tests)No further changes are needed.
ghost/i18n/locales/ca/portal.json (1)
107-107: New Localization Entry for Blocked Email Domains (Catalan)A new key
"Memberships from this email domain are currently restricted."has been added with an empty value. This is consistent with the PR’s intent to provide a specific error message when a user attempts to update to a blocked email domain. Please consider providing an appropriate Catalan translation in a follow-up update.ghost/i18n/locales/no/portal.json (1)
107-107: New Localization Entry for Blocked Email Domains (Norwegian)The key
"Memberships from this email domain are currently restricted."has been introduced with an empty translation. This new entry matches the requirements of the PR for informing users about restricted email domains. Ensure that a Norwegian translation is provided in a planned update if available.ghost/i18n/locales/hi/portal.json (1)
107-107: New Localization Entry for Blocked Email Domains (Hindi)The added key
"Memberships from this email domain are currently restricted."appears with an empty value. This aligns with the new blocked domain filter functionality. For improved user experience, please remember to update this entry with the proper Hindi translation when possible.ghost/i18n/locales/sq/portal.json (1)
107-107: New Localization Entry for Blocked Email Domains (Albanian)A new message key
"Memberships from this email domain are currently restricted."has been added with an empty string. This fulfills the update for displaying a clear error regarding restricted email domains. It would be beneficial to provide an Albanian translation in a future update when available.ghost/i18n/locales/sr-Cyrl/portal.json (1)
107-108: New Localization Entry for Blocked Email Domains (Serbian Cyrillic)The new key
"Memberships from this email domain are currently restricted."has been added with an empty translation. This is in line with the PR's objective to improve error messaging for blocked email domains. Please update this entry with the proper Serbian translation at your earliest convenience.ghost/core/test/e2e-api/members/send-magic-link.test.js (1)
380-399:❓ Verification inconclusive
Test implementation for blocked domains looks good.
The test effectively verifies that attempting to change a member's email to a blocked domain results in the expected error response and message.
However, there are two areas for potential improvement:
The comments on lines 394-396 about "Please sign up first" appear to be copied from another test and don't seem relevant to this test case.
Consider adding a complementary test that verifies a member can successfully change their email to a non-blocked domain, as suggested in a previous PR comment.
🏁 Script executed:
#!/bin/bash # Check if there's an existing test for changing email to non-blocked domain rg -A 3 "Allows changing email to a non-blocked email domain" ghost/core/test/e2e-api/members/Length of output: 96
Action Required: Remove Irrelevant Comment & Add Complementary Test for Non-Blocked Email Domain
- The test for blocked domains is implemented correctly.
- Remove the misleading comments on lines 394–396 regarding “Please sign up first” since they do not apply here.
- As no test currently verifies that a member can successfully change their email to a non-blocked domain, please consider adding a complementary test to ensure full coverage.
ghost/i18n/locales/gd/portal.json (1)
107-107: LGTM - New localization key added for blocked email domainsThe addition of this new localization key for "Memberships from this email domain are currently restricted." is appropriate and aligns with the PR objective of implementing blocked domain filtering for member email updates.
ghost/i18n/locales/lt/portal.json (1)
107-107: LGTM - Consistent localization key added across language filesThis matches the same localization key added in the Gaelic locale file, ensuring consistent messaging across languages when users attempt to update to a blocked email domain.
ghost/core/core/server/web/members/app.js (1)
64-64:✅ Verification successful
Proper error handling integration with middleware chain
Good improvement to pass the
nextparameter to theupdateEmailAddressmiddleware function. This allows error handling to be properly propagated through the Express middleware chain, which is essential for the new domain blocking functionality.
🏁 Script executed:
#!/bin/bash # Check if this middleware signature update is consistent with other similar middleware handlers # Look for other POST handlers in the same file that might need similar updates rg -A 2 "membersApp.post\(" ghost/core/core/server/web/members/app.jsLength of output: 1372
Middleware error handling integration confirmed
- The change to include the
nextparameter in theupdateEmailAddressmiddleware is consistent with other POST handlers in the file.- This implementation properly propagates errors through the Express middleware chain, aligning with the approach used in similar endpoints (e.g., for Stripe session creation).
No further changes are necessary.
ghost/members-api/lib/controllers/MemberController.js (6)
2-6: Good implementation of error message template.The use of the
tplmodule for the error message is consistent with Ghost's best practices for internationalization. The message is clear and descriptive for users.
18-18: Good documentation for the new dependency.The JSDoc comment update properly documents the new
settingsCachedependency parameter.
27-28: Correctly updated constructor signature to include settingsCache.The parameter has been added at the end of the parameter list, which is a good practice for maintaining backward compatibility when adding new parameters.
37-37: Properly initialized the settingsCache instance property.The
settingsCacheis correctly stored as an instance property with a consistent naming convention (this._settingsCache).
52-58: Well-implemented domain validation logic.The implementation:
- Retrieves blocked domains from settings cache
- Extracts the domain from the email with proper null checking using optional chaining
- Checks if the domain is in the blocked list
- Throws an appropriate error with a clear message
This implementation effectively prevents users from updating their email to blocked domains.
54-58:✅ Verification successful
Verify test coverage for blocked domain functionality.
Make sure there are tests covering both acceptance and rejection scenarios for this new validation logic.
🏁 Script executed:
#!/bin/bash # Check for test coverage of blocked email domain functionality # Search for tests related to blocked email domains echo "Looking for tests related to blocked email domains..." rg -A 3 "blocked.*domain|domain.*restrict" --glob "*.test.js"Length of output: 11213
Test coverage for blocked email domain functionality is well-verified.
The current e2e and unit tests, as seen in files like
ghost/core/test/e2e-api/members/send-magic-link.test.jsandghost/core/test/unit/server/services/settings-helpers/settings-helpers.test.js, ensure that both acceptance (allowed emails) and rejection (blocked emails) scenarios are covered. Please double-check that any additional edge cases—if identified later—are incorporated in future tests.apps/portal/src/actions.js (1)
486-490: Well-structured error handling for blocked email domains.The error handling logic properly detects the specific error message for blocked domains and displays an appropriate message to users. This provides a clear and specific explanation rather than a generic error message.
Note that the code correctly:
- Checks for the exact error message
- Uses
chooseBestErrorMessagefor consistent error handling- Falls back to generic messages for other error types
ghost/members-api/lib/members-api.js (2)
182-183: Correctly passed settingsCache to MemberController.The
settingsCacheis properly passed to theMemberControllerconstructor, enabling the email domain validation functionality.
182-183:✅ Verification successful
Verify settingsCache properly initializes blocked domains.
The code assumes that
settingsCache.get('all_blocked_email_domains')will return an array of blocked domains. Let's verify the structure of this setting to ensure it works correctly.
🏁 Script executed:
#!/bin/bash # Check how all_blocked_email_domains is initialized and used # Look for where all_blocked_email_domains is defined echo "Looking for all_blocked_email_domains definition..." rg -A 5 "all_blocked_email_domains" # Check for potential initialization issues or edge cases echo "Looking for potential initialization patterns..." rg -A 3 "settingsCache\.get\('all_blocked_email_domains'\)"Length of output: 13130
Blocked Domains Initialization Verified
The investigation confirms that
settingsCache.get('all_blocked_email_domains')is correctly initialized to return an array. The code in the settings service defines it using a calculated field withsettingsHelpers.getAllBlockedEmailDomains, and the associated tests—as seen in the snapshot and e2e tests—validate that it is correctly set (returning an array even when empty or populated).No changes are required here.
sagzy
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ghost/core/test/e2e-api/members/send-magic-link.test.js (2)
388-388: Consider explaining the identity field mock approachThe hardcoded identity value "12345678" is used here, which aligns with the PR comments about E2E tests not having a proper way to mock the identity field. Since this is a known issue mentioned in the past review comments, consider adding a code comment explaining why a hardcoded value is used here instead of generating a proper token (as done in the next test).
- identity: '12345678' + // Using hardcoded value since this test expects to fail before auth validation + identity: '12345678'
413-413: Consider verifying the member's email was actually updatedThe test currently only verifies the status code, but doesn't check if the member's email was actually changed in the database. Consider adding verification to ensure the email update was successful.
.expectStatus(201); + + // Verify the email was actually updated + const updatedMember = await membersService.api.members.get({id: member.id}); + should(updatedMember.get('email')).equal('[email protected]');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/portal/src/actions.js(2 hunks)ghost/core/test/e2e-api/members/send-magic-link.test.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/portal/src/actions.js
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Browser tests
🔇 Additional comments (2)
ghost/core/test/e2e-api/members/send-magic-link.test.js (2)
380-399: Great test for validating blocked domain email updates!The test correctly verifies the main PR objective of preventing emails from being updated to blocked domains. It checks for the appropriate 400 status code and validates the expected error message.
401-414: Good implementation of the suggested "non-blocked domain" testThis test properly validates that email changes to non-blocked domains succeed as expected. The use of a properly generated identity token is a good approach.
|
Deployed to staging with ID: |
…s their email address (#22320) closes https://linear.app/ghost/issue/ONC-797/spam-filter-not-applied-when-updating-an-email-address With this change, we extend the email domain blocklist to the member's email update feature (previously: only on signups) - Added logic in `MemberController.js` to validate email domains against a list of blocked domains from`settingsCache (all_blocked_email_domains)` before allowing updates. - Enhanced error messaging in `api.js` and `actions.js` in Portal to return a specific error ("Memberships from this email domain are currently restricted.") when a blocked domain error from API is detected. - Added a new E2E test to verify that changing an email to a blocked domain is prevented (status 400 with appropriate error message) and that changing an email to a allowed domain is still allowed (status 201) - Added the new error message to localisation files (portal.json) across multiple languages for consistent user feedback. --------- Co-authored-by: Sag <[email protected]>
…s their email address (#22320) closes https://linear.app/ghost/issue/ONC-797/spam-filter-not-applied-when-updating-an-email-address With this change, we extend the email domain blocklist to the member's email update feature (previously: only on signups) - Added logic in `MemberController.js` to validate email domains against a list of blocked domains from`settingsCache (all_blocked_email_domains)` before allowing updates. - Enhanced error messaging in `api.js` and `actions.js` in Portal to return a specific error ("Memberships from this email domain are currently restricted.") when a blocked domain error from API is detected. - Added a new E2E test to verify that changing an email to a blocked domain is prevented (status 400 with appropriate error message) and that changing an email to a allowed domain is still allowed (status 201) - Added the new error message to localisation files (portal.json) across multiple languages for consistent user feedback. --------- Co-authored-by: Sag <[email protected]>
ref https://linear.app/ghost/issue/ONC-797/spam-filter-not-applied-when-updating-an-email-address
With this change, we extend the email domain blocklist to member's email update feature (previously: only on signups)
MemberController.jsto validate email domains against a list of blocked domains fromsettingsCache (all_blocked_email_domains)before allowing updates.api.jsandactions.jsin Portal to return a specific error ("Memberships from this email domain are currently restricted.") when a blocked domain error from API is detected.