[fix] send email validate address - allow address with .afrca domain#4827
[fix] send email validate address - allow address with .afrca domain#4827MarshallRJ wants to merge 2 commits intoshesha-io:releases/0.43from
Conversation
…n and + in the mailbox name
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughSingle-file update to email validation and string utility methods: added a cached compiled email regex, return false for null/whitespace inputs, allow Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shesha-core/src/Shesha.Framework/Utilities/StringHelper.cs`:
- Around line 997-1017: The SnakeCaseTrim(string text, int length) overload
trims parts but returns string.Join("_", parts) which can leave double
underscores when parts become empty; update the return in SnakeCaseTrim(this
string text, int length) to normalize collapsed underscores (e.g., call
RemoveDoubleUnderscores() or ToSnakeCase()) to match the SnakeCaseTrim(string[]
parts, int length) behavior so both overloads produce identical snake-cased
output without consecutive underscores.
- Around line 867-875: IsValidEmail currently strips all internal whitespace
using Regex.Replace which can accept malformed addresses; change the
normalization to use inputEmail?.Trim() ?? "" (or remove normalization entirely)
in the IsValidEmail method, and make its regex a single static readonly Regex
(with RegexOptions.Compiled) instead of constructing a new Regex per call
(declare a new private static readonly Regex ValidEmailExpression and use it in
IsValidEmail). Also reconcile the two validators by updating the existing
EmailExpression / IsEmail to use the same pattern (or remove the duplicate
helper) so IsEmail and IsValidEmail share the canonical regex and behavior
(including allowing + and the updated TLD handling) to avoid divergent
validation paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: db80365a-1faf-4bf9-a781-fe377d4583a8
⛔ Files ignored due to path filters (1)
shesha-core/test/Shesha.Tests/StringHelper/StringHelperIsValidEmailTests.csis excluded by none and included by none
📒 Files selected for processing (1)
shesha-core/src/Shesha.Framework/Utilities/StringHelper.cs
…readonly as per code rabbit suggetion
|
@coderabbitai review changes |
|
✅ Actions performedReview triggered.
|
We are unable to send emails to address with the .africa domina eg: rob@test.africa
We are unable to send emails to address with + in the mailbox eg: rob+dep@test.com
We added test for these and amended the isValidEmail check
Summary by CodeRabbit
+character and allows longer TLDs; null/blank inputs are rejected.