Add comprehensive email validation for blocked users #8
Conversation
… many times each email address is blocked, and last time it was blocked. Move email validation out of User model and into EmailValidator. Signup form remembers which email addresses have failed and shows validation error on email field.
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
spec/models/blocked_email_spec.rb (1)
4-5: Optional: Clean up extra empty lines.RuboCop flags extra empty lines at the beginning and end of the describe block. These are minor style issues but could be cleaned up for consistency.
Apply this diff:
describe BlockedEmail do - let(:email) { '[email protected]' }end end - endAlso applies to: 47-48
spec/components/validators/email_validator_spec.rb (1)
4-5: Optional: Clean up extra empty lines.Similar to the blocked_email_spec, RuboCop flags extra empty lines at the beginning and end of the describe block.
Apply this diff:
describe EmailValidator do - let(:record) { Fabricate.build(:user, email: "[email protected]") }end - endAlso applies to: 22-23
app/assets/javascripts/discourse/controllers/create_account_controller.js (1)
69-74: Consider case-insensitive email comparison.The
rejectedEmails.contains(email)check on line 69 might be case-sensitive. If a user tries "[email protected]" after "[email protected]" failed, the client-side validation might not catch it. Consider normalizing the email to lowercase before the check and when pushing to rejectedEmails.Apply this diff:
email = this.get("accountEmail"); - if (this.get('rejectedEmails').contains(email)) { + if (this.get('rejectedEmails').contains(email.toLowerCase())) { return Discourse.InputValidation.create({And update line 275:
if (result.errors && result.errors.email && result.values) { - createAccountController.get('rejectedEmails').pushObject(result.values.email); + createAccountController.get('rejectedEmails').pushObject(result.values.email.toLowerCase()); }lib/validators/email_validator.rb (2)
18-22: Consider clarifying the expected format of the restriction setting.The method assumes
settingis a pipe-separated list of domains (since it's wrapped in parentheses for alternation in the regex). However, this format assumption is implicit and not documented.Consider adding a comment or method documentation:
+ # Checks if an email matches domains in a pipe-separated setting string + # @param setting [String] Pipe-separated domains (e.g., "example.com|test.org") + # @param value [String] Email address to check + # @return [Boolean] True if email domain matches any domain in setting def email_in_restriction_setting?(setting, value) domains = Regexp.escape(setting) regexp = Regexp.new("@(#{domains})", true) value =~ regexp endAlternatively, you could make the return value more explicit:
def email_in_restriction_setting?(setting, value) domains = Regexp.escape(setting) regexp = Regexp.new("@(#{domains})", true) - value =~ regexp + !!(value =~ regexp) end
13-13: Consider using&&instead ofandfor consistency.Ruby's
andoperator has very low precedence and is less commonly used in conditionals. The more idiomatic choice is&&.- if record.errors[attribute].blank? and BlockedEmail.should_block?(value) + if record.errors[attribute].blank? && BlockedEmail.should_block?(value)app/models/blocked_email.rb (1)
11-11: Consider renaming method to indicate side effects.The method name
should_block?follows Ruby's predicate method convention (ending with?), which implies it's a pure query method without side effects. However, this method mutates the database by incrementing counters and updating timestamps, which is unexpected behavior for a predicate method.Consider one of these approaches:
Option 1: Rename to indicate side effects:
- def self.should_block?(email) + def self.check_and_track_blocked(email)Option 2: Split into two methods (preferable - see comment on email_validator.rb):
def self.blocked?(email) record = BlockedEmail.find_by(email: email) record&.action_type == actions[:block] end def self.track_block_attempt(email) record = BlockedEmail.find_by(email: email) return unless record record.increment!(:match_count) record.update_column(:last_match_at, Time.zone.now) endThis separation makes the code more testable and follows the single responsibility principle.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/assets/javascripts/discourse/controllers/create_account_controller.js(4 hunks)app/controllers/users_controller.rb(1 hunks)app/models/blocked_email.rb(1 hunks)app/models/user.rb(1 hunks)config/locales/server.en.yml(1 hunks)db/migrate/20130724201552_create_blocked_emails.rb(1 hunks)lib/validators/email_validator.rb(1 hunks)spec/components/validators/email_validator_spec.rb(1 hunks)spec/fabricators/blocked_email_fabricator.rb(1 hunks)spec/models/blocked_email_spec.rb(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
spec/models/blocked_email_spec.rb (2)
app/models/blocked_email.rb (2)
actions(7-9)should_block?(11-19)db/migrate/20130724201552_create_blocked_emails.rb (2)
change(1-12)change(2-11)
lib/validators/email_validator.rb (1)
app/models/blocked_email.rb (1)
should_block?(11-19)
spec/components/validators/email_validator_spec.rb (1)
lib/validators/email_validator.rb (2)
validate_each(1-24)validate_each(3-16)
spec/fabricators/blocked_email_fabricator.rb (1)
app/models/blocked_email.rb (1)
actions(7-9)
🪛 RuboCop (1.81.7)
spec/models/blocked_email_spec.rb
[convention] 4-5: Extra empty line detected at block body beginning.
(Layout/EmptyLinesAroundBlockBody)
[convention] 47-48: Extra empty line detected at block body end.
(Layout/EmptyLinesAroundBlockBody)
spec/components/validators/email_validator_spec.rb
[convention] 4-5: Extra empty line detected at block body beginning.
(Layout/EmptyLinesAroundBlockBody)
[convention] 22-23: Extra empty line detected at block body end.
(Layout/EmptyLinesAroundBlockBody)
🔇 Additional comments (9)
config/locales/server.en.yml (1)
734-734: LGTM!The locale entry for blocked emails is clear and follows the existing pattern.
spec/fabricators/blocked_email_fabricator.rb (1)
1-4: LGTM!The fabricator is well-structured with a sequence for unique emails and a sensible default action_type.
app/controllers/users_controller.rb (1)
197-199: Consider information disclosure implications.Returning
user.attributes.slice("name", "username", "email")in the error response allows clients to know which specific email was rejected. While this improves UX for the client-siderejectedEmailstracking, it could potentially enable enumeration of blocked emails by malicious actors.Consider whether this trade-off is acceptable for your security model, or if you should only return values for non-email-related errors.
db/migrate/20130724201552_create_blocked_emails.rb (1)
1-12: LGTM!The migration structure is solid with appropriate columns, constraints, and a unique index on email.
spec/models/blocked_email_spec.rb (1)
1-48: Excellent test coverage!The tests comprehensively cover default behavior, blocking logic, statistics tracking, and different action types.
spec/components/validators/email_validator_spec.rb (1)
1-23: Tests are clear and focused.The validator tests appropriately stub BlockedEmail.should_block? to isolate the validation logic. Good separation of concerns.
app/assets/javascripts/discourse/controllers/create_account_controller.js (1)
17-17: Excellent client-side validation enhancement!The rejectedEmails tracking prevents repeated submissions of blocked emails and provides immediate feedback. The dependency declaration ensures re-validation when the array changes.
Also applies to: 96-96, 274-276
app/models/user.rb (1)
46-47: Excellent refactoring!The validation logic is now cleaner and follows Rails conventions. Moving the domain restriction and blocking logic to EmailValidator provides better separation of concerns and makes the model more maintainable.
app/models/blocked_email.rb (1)
7-9: Enum class implementation is properly verified and supports hash-like access.The
Enumclass inlib/enum.rbis a custom implementation that extendsHash. The constructor creates a hash mapping symbols to numeric values usingHash[members.zip(start..members.count + start)], defaulting to start at 1. The overridden[]method provides bidirectional access: given a symbol returns its numeric value, given a number returns its symbol. The test suite confirms this pattern works correctly, including the exact usage inblocked_email.rbline 8 (actions[:block]returns 1). No issues found.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis pull request implements an email blocking system by introducing a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller as Create Account<br/>Controller
participant UsersCtrl as Users<br/>Controller
participant UserModel as User Model
participant Validator as Email<br/>Validator
participant BlockedEmail as Blocked<br/>Email
User->>Controller: Submit account form
Controller->>Controller: emailValidation check<br/>(rejects if in<br/>rejectedEmails)
alt Email invalid locally
Controller->>User: Show validation error
else Email passes local check
Controller->>UsersCtrl: POST create account
UsersCtrl->>UserModel: Create user record
UserModel->>Validator: Validate email
Validator->>BlockedEmail: should_block?(email)
alt Email is blocked
BlockedEmail->>BlockedEmail: Increment match_count<br/>Update last_match_at
BlockedEmail-->>Validator: true
Validator-->>UserModel: Add error
UserModel-->>UsersCtrl: Save fails
UsersCtrl->>UsersCtrl: Build error response<br/>(errors hash + values)
UsersCtrl-->>Controller: JSON errors
Controller->>Controller: Push email to<br/>rejectedEmails
Controller->>User: Show error
else Email allowed
BlockedEmail-->>Validator: false
Validator-->>UserModel: No error
UserModel->>UserModel: Save successful
UserModel-->>UsersCtrl: Success
UsersCtrl-->>Controller: Success response
Controller-->>User: Account created
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
db/migrate/20130724201552_create_blocked_emails.rb (1)
1-12: LGTM! Consider adding an index on action_type for query performance.The migration structure is solid. The unique index on email is essential for lookups and preventing duplicates.
If you frequently query blocked emails by action_type, consider adding an index:
add_index :blocked_emails, :email, unique: true + add_index :blocked_emails, :action_typeapp/assets/javascripts/discourse/controllers/create_account_controller.js (1)
69-74: Consider using a more specific error message for blocked emails.The validation correctly prevents re-submission of rejected emails. However, using
I18n.t('user.email.invalid')might be misleading since the email format could be valid but the email itself is blocked.Consider using the new
user.email.blockedtranslation key added in this PR:if (this.get('rejectedEmails').contains(email)) { return Discourse.InputValidation.create({ failed: true, - reason: I18n.t('user.email.invalid') + reason: I18n.t('user.email.blocked') }); }lib/validators/email_validator.rb (1)
18-22: Improve regex construction to prevent potential issues with special characters.The method correctly handles pipe-delimited domains and case-insensitive matching. However, it only escapes dots, leaving other regex special characters (e.g.,
+,*,?,[,]) unescaped.While admin-controlled settings are generally trusted, it's safer to use
Regexp.escapefor the entire domain string:def email_in_restriction_setting?(setting, value) - domains = setting.gsub('.', '\.') + domains = setting.split('|').map { |d| Regexp.escape(d.strip) }.join('|') regexp = Regexp.new("@(#{domains})", true) value =~ regexp endThis approach:
- Escapes all regex special characters
- Handles whitespace around pipe delimiters
- Prevents potential regex injection if domain names contain special chars
config/locales/server.en.yml (1)
734-734: Consider making the blocked email message more descriptive.The translation is clear and concise. However, comparing it to the
not_allowedmessage above (line 733), theblockedmessage could provide more context to help users understand why their email was rejected.Consider making the message more informative:
- blocked: "is not allowed." + blocked: "has been blocked. Please use a different email address."spec/models/blocked_email_spec.rb (1)
1-48: Comprehensive test coverage for BlockedEmail model.The specs thoroughly test the model's behavior including defaults, blocking logic, and usage statistics. The use of shared examples and Timecop demonstrates good testing practices.
Optional: Address RuboCop style hints by removing extra blank lines at block boundaries (lines 4-5 and 47-48) for consistency with the project's style guide.
spec/components/validators/email_validator_spec.rb (1)
1-23: Tests cover BlockedEmail integration but missing whitelist/blacklist coverage.The specs correctly verify the validator's interaction with
BlockedEmail.should_block?. However, the test suite should also cover the whitelist and blacklist logic implemented inEmailValidator.Consider adding test coverage for:
- Email domain whitelist behavior (allowed and rejected domains)
- Email domain blacklist behavior (blocked domains)
- Whitelist taking precedence over blacklist
- Edge cases (empty settings, malformed domains)
Example test structure:
context "whitelist" do before { SiteSetting.stubs(:email_domains_whitelist).returns("allowed.com") } it "allows email from whitelisted domain" do record.email = "[email protected]" validate record.errors[:email].should_not be_present end it "blocks email not from whitelisted domain" do record.email = "[email protected]" validate record.errors[:email].should be_present end endOptional: Address RuboCop style hints by removing extra blank lines at block boundaries (lines 4-5 and 22-23).
app/models/blocked_email.rb (2)
7-9: Consider using Rails' built-in enum macro.The custom
Enum.newpattern works but is non-standard. Rails provides a built-inenummacro that offers better integration, query scopes, and validation.Apply this refactor to use Rails enum:
- def self.actions - @actions ||= Enum.new(:block, :do_nothing) - end + enum action_type: { + block: 0, + do_nothing: 1 + }This provides automatic scopes like
BlockedEmail.blockand predicate methods likerecord.block?.
21-23: Consider alternative default-setting approaches.While the current implementation works, Rails offers more idiomatic alternatives:
- Database default: Set in the migration for better data integrity.
after_initializecallback: More appropriate for object initialization defaults.Alternative implementations:
Option 1: Database default (recommended)
In the migration:t.integer :action_type, default: 0, null: falseOption 2: after_initialize callback
- before_validation :set_defaults + after_initialize :set_defaults, if: :new_record?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/assets/javascripts/discourse/controllers/create_account_controller.js(4 hunks)app/controllers/users_controller.rb(1 hunks)app/models/blocked_email.rb(1 hunks)app/models/user.rb(1 hunks)config/locales/server.en.yml(1 hunks)db/migrate/20130724201552_create_blocked_emails.rb(1 hunks)lib/validators/email_validator.rb(1 hunks)spec/components/validators/email_validator_spec.rb(1 hunks)spec/fabricators/blocked_email_fabricator.rb(1 hunks)spec/models/blocked_email_spec.rb(1 hunks)
🧰 Additional context used
🪛 RuboCop (1.81.7)
spec/components/validators/email_validator_spec.rb
[convention] 4-5: Extra empty line detected at block body beginning.
(Layout/EmptyLinesAroundBlockBody)
[convention] 22-23: Extra empty line detected at block body end.
(Layout/EmptyLinesAroundBlockBody)
spec/models/blocked_email_spec.rb
[convention] 4-5: Extra empty line detected at block body beginning.
(Layout/EmptyLinesAroundBlockBody)
[convention] 47-48: Extra empty line detected at block body end.
(Layout/EmptyLinesAroundBlockBody)
🔇 Additional comments (6)
app/assets/javascripts/discourse/controllers/create_account_controller.js (2)
17-17: Good addition for client-side validation feedback.The
rejectedEmailsarray effectively prevents users from immediately re-submitting blocked emails. Since the controller is likely recreated when the modal reopens, this approach avoids memory leaks.
274-276: Solid error handling with proper defensive checks.The conditional correctly verifies all necessary properties exist before adding the email to
rejectedEmails. Usingresult.values.emailfrom the server response ensures the exact value that failed validation is tracked.app/models/user.rb (1)
46-47: Clean validation consolidation with proper separation of concerns.The refactored email validations are more maintainable by delegating domain restriction and blocked email checks to the
EmailValidator. The conditionalif: :email_changed?efficiently avoids redundant validation.app/controllers/users_controller.rb (1)
195-200: Well-structured error response for client-side validation feedback.The enhanced response correctly provides both field-level errors and the attempted values, enabling the client-side
rejectedEmailstracking mechanism. Exposing the email value is appropriate in this context for validation feedback.spec/fabricators/blocked_email_fabricator.rb (1)
1-4: LGTM! Clean test fabricator.The fabricator correctly generates unique test emails with a clear pattern and properly references the model's action enum.
app/models/blocked_email.rb (1)
5-5: Database-level uniqueness constraint confirmed.The unique index exists on the
db/migrate/20130724201552_create_blocked_emails.rb:10). Combined with the application-level validation, this provides adequate protection against race conditions during concurrent inserts.
| def self.should_block?(email) | ||
| record = BlockedEmail.where(email: email).first | ||
| if record | ||
| record.match_count += 1 | ||
| record.last_match_at = Time.zone.now | ||
| record.save | ||
| end | ||
| record && record.action_type == actions[:block] | ||
| end |
There was a problem hiding this comment.
Critical: Race condition and silent failure in should_block?
This method has several critical issues:
-
Race condition (lines 14-16): Multiple concurrent calls with the same email will read the same
match_count, each increment it, and each save, resulting in lost increments. This is a classic TOCTOU (time-of-check-time-of-use) vulnerability. -
Silent failure (line 16): The
savecall can fail due to validation errors or database constraints, but failures are silently ignored. Tracking data may be lost while the method still returns a blocking decision. -
Misleading method name (lines 11-19): The name
should_block?suggests a read-only query, but it modifies the database. This violates the principle of least surprise and makes the code harder to reason about.
Apply this diff to fix the race condition and improve reliability:
def self.should_block?(email)
- record = BlockedEmail.where(email: email).first
+ record = BlockedEmail.find_by(email: email)
if record
- record.match_count += 1
- record.last_match_at = Time.zone.now
- record.save
+ # Use atomic increment to prevent race conditions
+ record.increment!(:match_count)
+ record.update_column(:last_match_at, Time.zone.now)
end
record && record.action_type == actions[:block]
endNote: Consider separating the blocking check from the tracking logic to make the side effects more explicit, e.g., a separate track_match!(email) method.
🤖 Prompt for AI Agents
In app/models/blocked_email.rb around lines 11 to 19, the should_block? method
both mutates state and returns a boolean which causes a race condition and
silent failures; refactor by splitting responsibilities: rename should_block? to
a pure predicate that only reads (e.g., blocked?), create a separate
track_match!(email) method that performs an atomic DB-level increment and
timestamp update (use update_counters or an UPDATE ... SET match_count =
match_count + 1, last_match_at = ? with a where clause inside a transaction or
use incremental SQL to avoid TOCTOU), ensure the tracking method raises or logs
on failure (do not silently ignore save failures), and update callers to call
blocked? for checks and track_match! when you want to record a match so behavior
and side effects are explicit.
| def validate_each(record, attribute, value) | ||
| if (setting = SiteSetting.email_domains_whitelist).present? | ||
| unless email_in_restriction_setting?(setting, value) | ||
| record.errors.add(attribute, I18n.t(:'user.email.not_allowed')) | ||
| end | ||
| elsif (setting = SiteSetting.email_domains_blacklist).present? | ||
| if email_in_restriction_setting?(setting, value) | ||
| record.errors.add(attribute, I18n.t(:'user.email.not_allowed')) | ||
| end | ||
| end | ||
| if record.errors[attribute].blank? and BlockedEmail.should_block?(value) | ||
| record.errors.add(attribute, I18n.t(:'user.email.blocked')) | ||
| end | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
Add basic email format validation before domain and blocked email checks.
The validator correctly handles whitelist/blacklist precedence and blocked email checks. However, it appears to be missing basic email format validation (e.g., presence of @, valid domain format).
Without format validation, malformed emails like "not-an-email" could reach the BlockedEmail.should_block? check. Verify whether email format validation occurs elsewhere in the validation chain:
If no format validation exists, consider adding it:
def validate_each(record, attribute, value)
+ # Basic format check
+ unless value =~ /\A[^@\s]+@[^@\s]+\.[^@\s]+\z/
+ record.errors.add(attribute, I18n.t(:'user.email.invalid'))
+ return
+ end
+
if (setting = SiteSetting.email_domains_whitelist).present?🏁 Script executed:
#!/bin/bash
# Search for email format validation in the User model or other validators
rg -n -B3 -A3 'email.*format|format.*email|emailValid|email.*regexp|EMAIL.*=' --type=rb -g '!spec/**'Length of output: 26
🏁 Script executed:
#!/bin/bash
# Search for email format validation patterns without type restriction
rg -n 'email.*format|emailValid|EMAIL.*REGEX|@.*format' -g '!spec/**' -g '!test/**'Length of output: 318677
🏁 Script executed:
#!/bin/bash
# Find email validator and related files
fd -e rb 'email|validator' | head -20Length of output: 957
🏁 Script executed:
#!/bin/bash
# Search for BlockedEmail class and should_block? method
rg -n 'class BlockedEmail|def should_block' -g '!spec/**' -g '!test/**'Length of output: 69
🏁 Script executed:
#!/bin/bash
# Search for User model email validation
rg -n 'validates.*:email|email.*validates' -g '!spec/**' -g '!test/**'Length of output: 120339
🏁 Script executed:
#!/bin/bash
set -e
echo "== lib/validators/email_validator.rb =="
if [ -f lib/validators/email_validator.rb ]; then sed -n '1,240p' lib/validators/email_validator.rb; else echo "MISSING: lib/validators/email_validator.rb"; fi
echo
echo "== app/models/blocked_email.rb =="
if [ -f app/models/blocked_email.rb ]; then sed -n '1,240p' app/models/blocked_email.rb; else echo "MISSING: app/models/blocked_email.rb"; fi
echo
echo "== app/models/user.rb (first 200 lines) =="
if [ -f app/models/user.rb ]; then sed -n '1,200p' app/models/user.rb; else echo "MISSING: app/models/user.rb"; fi
echo
echo "== rg for should_block? =="
rg -n "should_block\\?" || trueLength of output: 7484
Validate email format before domain/blocked checks.
Add a simple format check at the top of lib/validators/email_validator.rb#validate_each so malformed emails are rejected before whitelist/blacklist/blocked checks (BlockedEmail.should_block? assumes a valid address).
Suggested minimal change:
def validate_each(record, attribute, value)
+ # Basic format check
+ unless value =~ /\A[^@\s]+@[^@\s]+\.[^@\s]+\z/
+ record.errors.add(attribute, I18n.t(:'user.email.invalid'))
+ return
+ endlib/validators/email_validator.rb — update validate_each.
📝 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.
| def validate_each(record, attribute, value) | |
| if (setting = SiteSetting.email_domains_whitelist).present? | |
| unless email_in_restriction_setting?(setting, value) | |
| record.errors.add(attribute, I18n.t(:'user.email.not_allowed')) | |
| end | |
| elsif (setting = SiteSetting.email_domains_blacklist).present? | |
| if email_in_restriction_setting?(setting, value) | |
| record.errors.add(attribute, I18n.t(:'user.email.not_allowed')) | |
| end | |
| end | |
| if record.errors[attribute].blank? and BlockedEmail.should_block?(value) | |
| record.errors.add(attribute, I18n.t(:'user.email.blocked')) | |
| end | |
| end | |
| def validate_each(record, attribute, value) | |
| # Basic format check | |
| unless value =~ /\A[^@\s]+@[^@\s]+\.[^@\s]+\z/ | |
| record.errors.add(attribute, I18n.t(:'user.email.invalid')) | |
| return | |
| end | |
| if (setting = SiteSetting.email_domains_whitelist).present? | |
| unless email_in_restriction_setting?(setting, value) | |
| record.errors.add(attribute, I18n.t(:'user.email.not_allowed')) | |
| end | |
| elsif (setting = SiteSetting.email_domains_blacklist).present? | |
| if email_in_restriction_setting?(setting, value) | |
| record.errors.add(attribute, I18n.t(:'user.email.not_allowed')) | |
| end | |
| end | |
| if record.errors[attribute].blank? and BlockedEmail.should_block?(value) | |
| record.errors.add(attribute, I18n.t(:'user.email.blocked')) | |
| end | |
| end |
🤖 Prompt for AI Agents
In lib/validators/email_validator.rb around lines 3 to 16, validate_each
currently runs whitelist/blacklist and BlockedEmail checks without first
ensuring the email is well-formed; add a simple format check at the top of
validate_each that uses a standard email regex (e.g. Ruby's
URI::MailTo::EMAIL_REGEXP or a minimal reasonable regex) and if the value is
malformed call record.errors.add(attribute, I18n.t(:'user.email.invalid')) (or
an existing invalid-email i18n key) and return early so subsequent
whitelist/blacklist/BlockedEmail.should_block? logic only runs for valid
addresses.
Test 3
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.
Replicated from ai-code-review-evaluation/discourse-coderabbit#3