[feature] EmailChannelSender allow for send from email#4826
[feature] EmailChannelSender allow for send from email#4826MarshallRJ wants to merge 1 commit intoshesha-io:releases/0.43from
Conversation
WalkthroughThe PR adds a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant EmailSender as EmailChannelSender
participant Settings as SmtpSettings
Client->>EmailSender: BuildMessageWith(fromAddress, smtpSettings)
EmailSender->>Settings: Check ForceFromAddressFromSettings
alt fromAddress is null/empty OR ForceFromAddressFromSettings is true
Settings->>EmailSender: ForceFromAddressFromSettings = true
alt UseSmtpRelay enabled
Settings->>EmailSender: DefaultFromAddress + DisplayName
EmailSender->>EmailSender: Set From with DefaultFromAddress
else UseSmtpRelay disabled
Settings->>EmailSender: UserName
EmailSender->>EmailSender: Set From with UserName
end
else fromAddress provided AND ForceFromAddressFromSettings is false
EmailSender->>EmailSender: Set From with provided fromAddress
end
EmailSender->>Client: Return configured MailMessage
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.Application/Notifications/Emails/EmailChannelSender.cs`:
- Around line 143-161: When ForceFromAddressFromSettings is true,
EmailChannelSender must always use smtpSettings.DefaultFromAddress as the
message.From regardless of smtpSettings.UseSmtpRelay; update the block in
EmailChannelSender so it selects DefaultFromAddress unconditionally when
ForceFromAddressFromSettings==true and throw a clear InvalidOperationException
if DefaultFromAddress is null/empty (do not fall back to smtpSettings.UserName).
Also ensure you do not construct a MailAddress from UserName in this forced
branch. Finally, review SheshaEmailSender.NormalizeMailSender to either honor
ForceFromAddressFromSettings the same way or document that the flag applies only
to EmailChannelSender and adjust behavior accordingly.
🪄 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: ff0685fe-d001-48e3-8467-c5de38e4937b
📒 Files selected for processing (4)
shesha-core/src/Shesha.Application/Email/Dtos/SmtpSettingsDto.csshesha-core/src/Shesha.Application/Email/EmailSenderAppService.csshesha-core/src/Shesha.Application/Notifications/Emails/EmailChannelSender.csshesha-core/src/Shesha.Framework/Configuration/Email/SmtpSettings.cs
We are using Sendgrid as our smtp email sender. There is no way to force the emails to have the defaultFromAddress forced to be set as what is configured in the SMTP settings. If smtpRelay is set we use the defaultFromAddress and if not we use the configured smtp username (This was the functioning in the obsolete SheshaEmailSender)
We have added this feature
Summary by CodeRabbit