Fix contact sales form: add SMTP email notifications on inquiry submission#435
Conversation
- Add IEmailNotificationService interface and SmtpEmailNotificationService implementation - Update SalesInquiryService to call email service after saving inquiry to DB - Register SmtpEmailNotificationService as singleton in Program.cs - Add Email SMTP config section to appsettings.json - Add 3 unit tests for SmtpEmailNotificationService Co-authored-by: aurelianware <194855645+aurelianware@users.noreply.github.com>
|
@colilot do I need to configure a mail account to send from? |
There was a problem hiding this comment.
Pull request overview
Adds SMTP-based email notifications for the Portal’s “Contact Sales” flow so that creating a sales inquiry triggers (1) an internal alert to the sales team and (2) a confirmation email to the submitter, while keeping existing behavior when SMTP isn’t configured.
Changes:
- Introduces
IEmailNotificationServiceand an SMTP-backed implementation (SmtpEmailNotificationService) to send sales inquiry notifications. - Updates
SalesInquiryService.CreateInquiryAsync()to send notifications after successfully persisting the inquiry to MongoDB. - Adds
Emailconfiguration defaults and registers the new email service in DI; includes unit tests for basic failure/no-config behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/portal/CloudHealthOffice.Portal/appsettings.json | Adds Email configuration block (SMTP host/port/SSL/from/sales team creds). |
| src/portal/CloudHealthOffice.Portal/Services/ServiceImplementations.cs | Injects email notifications into inquiry creation and adds SmtpEmailNotificationService implementation. |
| src/portal/CloudHealthOffice.Portal/Services/IServices.cs | Adds IEmailNotificationService interface. |
| src/portal/CloudHealthOffice.Portal/Program.cs | Registers SmtpEmailNotificationService as the IEmailNotificationService implementation. |
| src/portal/CloudHealthOffice.Portal.Tests/Services/SmtpEmailNotificationServiceTests.cs | Adds tests for “SMTP not configured” and “SMTP failure doesn’t throw” scenarios. |
| var smtpPort = int.TryParse(_configuration["Email:SmtpPort"], out var port) ? port : 587; | ||
| var enableSsl = !string.Equals(_configuration["Email:EnableSsl"], "false", StringComparison.OrdinalIgnoreCase); | ||
| var fromAddress = _configuration["Email:FromAddress"] ?? "noreply@cloudhealthoffice.com"; | ||
| var salesTeamAddress = _configuration["Email:SalesTeamAddress"] ?? "sales@cloudhealthoffice.com"; | ||
| var username = _configuration["Email:Username"]; | ||
| var password = _configuration["Email:Password"]; | ||
|
|
||
| using var client = new SmtpClient(smtpHost, smtpPort) | ||
| { | ||
| EnableSsl = enableSsl, | ||
| Credentials = !string.IsNullOrWhiteSpace(username) | ||
| ? new NetworkCredential(username, password) | ||
| : CredentialCache.DefaultNetworkCredentials | ||
| }; | ||
|
|
||
| try | ||
| { | ||
| using var salesNotification = BuildSalesTeamEmail(fromAddress, salesTeamAddress, inquiry); | ||
| await client.SendMailAsync(salesNotification); | ||
| _logger.LogInformation("Sales team notification sent for inquiry {InquiryId}", inquiry.Id); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Failed to send sales team notification for inquiry {InquiryId}", inquiry.Id); | ||
| // Do not rethrow — email failure must not prevent a successful inquiry submission | ||
| } | ||
|
|
||
| if (!string.IsNullOrWhiteSpace(inquiry.Email) && IsValidEmail(inquiry.Email)) | ||
| { | ||
| try | ||
| { | ||
| using var confirmation = BuildConfirmationEmail(fromAddress, inquiry); | ||
| await client.SendMailAsync(confirmation); | ||
| _logger.LogInformation("Confirmation email sent to {Email} for inquiry {InquiryId}", inquiry.Email, inquiry.Id); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Failed to send confirmation email to {Email} for inquiry {InquiryId}", inquiry.Email, inquiry.Id); | ||
| // Do not rethrow — confirmation email failure must not prevent a successful inquiry submission | ||
| } | ||
| } |
There was a problem hiding this comment.
smtpPort parsing accepts any int; invalid values (e.g., -1 or > 65535) will cause SmtpClient construction to throw and bubble up. Clamp/validate the range and default to 587 on invalid values, or wrap the client construction in a try/catch so misconfiguration can’t break inquiry submission.
| var smtpPort = int.TryParse(_configuration["Email:SmtpPort"], out var port) ? port : 587; | |
| var enableSsl = !string.Equals(_configuration["Email:EnableSsl"], "false", StringComparison.OrdinalIgnoreCase); | |
| var fromAddress = _configuration["Email:FromAddress"] ?? "noreply@cloudhealthoffice.com"; | |
| var salesTeamAddress = _configuration["Email:SalesTeamAddress"] ?? "sales@cloudhealthoffice.com"; | |
| var username = _configuration["Email:Username"]; | |
| var password = _configuration["Email:Password"]; | |
| using var client = new SmtpClient(smtpHost, smtpPort) | |
| { | |
| EnableSsl = enableSsl, | |
| Credentials = !string.IsNullOrWhiteSpace(username) | |
| ? new NetworkCredential(username, password) | |
| : CredentialCache.DefaultNetworkCredentials | |
| }; | |
| try | |
| { | |
| using var salesNotification = BuildSalesTeamEmail(fromAddress, salesTeamAddress, inquiry); | |
| await client.SendMailAsync(salesNotification); | |
| _logger.LogInformation("Sales team notification sent for inquiry {InquiryId}", inquiry.Id); | |
| } | |
| catch (Exception ex) | |
| { | |
| _logger.LogError(ex, "Failed to send sales team notification for inquiry {InquiryId}", inquiry.Id); | |
| // Do not rethrow — email failure must not prevent a successful inquiry submission | |
| } | |
| if (!string.IsNullOrWhiteSpace(inquiry.Email) && IsValidEmail(inquiry.Email)) | |
| { | |
| try | |
| { | |
| using var confirmation = BuildConfirmationEmail(fromAddress, inquiry); | |
| await client.SendMailAsync(confirmation); | |
| _logger.LogInformation("Confirmation email sent to {Email} for inquiry {InquiryId}", inquiry.Email, inquiry.Id); | |
| } | |
| catch (Exception ex) | |
| { | |
| _logger.LogError(ex, "Failed to send confirmation email to {Email} for inquiry {InquiryId}", inquiry.Email, inquiry.Id); | |
| // Do not rethrow — confirmation email failure must not prevent a successful inquiry submission | |
| } | |
| } | |
| var smtpPortConfig = _configuration["Email:SmtpPort"]; | |
| int smtpPort; | |
| if (!int.TryParse(smtpPortConfig, out smtpPort) || smtpPort <= 0 || smtpPort > 65535) | |
| { | |
| _logger.LogWarning("Invalid SMTP port configuration value '{SmtpPort}'. Defaulting to 587.", smtpPortConfig); | |
| smtpPort = 587; | |
| } | |
| var enableSsl = !string.Equals(_configuration["Email:EnableSsl"], "false", StringComparison.OrdinalIgnoreCase); | |
| var fromAddress = _configuration["Email:FromAddress"] ?? "noreply@cloudhealthoffice.com"; | |
| var salesTeamAddress = _configuration["Email:SalesTeamAddress"] ?? "sales@cloudhealthoffice.com"; | |
| var username = _configuration["Email:Username"]; | |
| var password = _configuration["Email:Password"]; | |
| try | |
| { | |
| using var client = new SmtpClient(smtpHost, smtpPort) | |
| { | |
| EnableSsl = enableSsl, | |
| Credentials = !string.IsNullOrWhiteSpace(username) | |
| ? new NetworkCredential(username, password) | |
| : CredentialCache.DefaultNetworkCredentials | |
| }; | |
| try | |
| { | |
| using var salesNotification = BuildSalesTeamEmail(fromAddress, salesTeamAddress, inquiry); | |
| await client.SendMailAsync(salesNotification); | |
| _logger.LogInformation("Sales team notification sent for inquiry {InquiryId}", inquiry.Id); | |
| } | |
| catch (Exception ex) | |
| { | |
| _logger.LogError(ex, "Failed to send sales team notification for inquiry {InquiryId}", inquiry.Id); | |
| // Do not rethrow — email failure must not prevent a successful inquiry submission | |
| } | |
| if (!string.IsNullOrWhiteSpace(inquiry.Email) && IsValidEmail(inquiry.Email)) | |
| { | |
| try | |
| { | |
| using var confirmation = BuildConfirmationEmail(fromAddress, inquiry); | |
| await client.SendMailAsync(confirmation); | |
| _logger.LogInformation("Confirmation email sent to {Email} for inquiry {InquiryId}", inquiry.Email, inquiry.Id); | |
| } | |
| catch (Exception ex) | |
| { | |
| _logger.LogError(ex, "Failed to send confirmation email to {Email} for inquiry {InquiryId}", inquiry.Email, inquiry.Id); | |
| // Do not rethrow — confirmation email failure must not prevent a successful inquiry submission | |
| } | |
| } | |
| } | |
| catch (Exception ex) | |
| { | |
| _logger.LogError(ex, "Failed to initialize SMTP client for inquiry {InquiryId}", inquiry.Id); | |
| // Do not rethrow — email failure (including client initialization) must not prevent a successful inquiry submission | |
| } |
|
|
||
| _logger.LogInformation("Successfully created sales inquiry {InquiryId}", inquiryId); | ||
|
|
||
| await _emailNotificationService.SendSalesInquiryNotificationAsync(inquiry); |
There was a problem hiding this comment.
CreateInquiryAsync awaits SendSalesInquiryNotificationAsync inside the main try/catch; if email sending throws (e.g., due to SMTP port misconfiguration) the method will rethrow even though the inquiry was already inserted, leading the UI to show a failure while the inquiry actually exists. Wrap the notification call in its own try/catch and always return inquiryId after a successful DB insert.
| await _emailNotificationService.SendSalesInquiryNotificationAsync(inquiry); | |
| try | |
| { | |
| await _emailNotificationService.SendSalesInquiryNotificationAsync(inquiry); | |
| } | |
| catch (Exception notificationEx) | |
| { | |
| _logger.LogError(notificationEx, | |
| "Failed to send sales inquiry notification for {InquiryId}", inquiryId); | |
| } |
| public async Task SendSalesInquiryNotificationAsync_WhenSmtpFails_DoesNotThrow() | ||
| { | ||
| // Arrange — point at a non-existent SMTP server to force a send failure | ||
| var config = BuildConfiguration(new Dictionary<string, string?> | ||
| { | ||
| ["Email:SmtpHost"] = "smtp.invalid.example.test", | ||
| ["Email:SmtpPort"] = "587", | ||
| ["Email:EnableSsl"] = "false", | ||
| ["Email:FromAddress"] = "noreply@cloudhealthoffice.com", | ||
| ["Email:SalesTeamAddress"] = "sales@cloudhealthoffice.com" | ||
| }); | ||
| var sut = new SmtpEmailNotificationService(config, _logger.Object); | ||
|
|
||
| // Act | ||
| var exception = await Record.ExceptionAsync(() => | ||
| sut.SendSalesInquiryNotificationAsync(BuildSampleInquiry())); | ||
|
|
There was a problem hiding this comment.
This test forces SMTP failure by calling a real host name; depending on DNS/network conditions, SendMailAsync can take a long time (default SmtpClient timeout is ~100s) and make CI flaky. Prefer a deterministic failure (e.g., localhost with a closed port) and/or add a configurable SmtpClient timeout so tests can set it low.
| Body = body, | ||
| IsBodyHtml = false | ||
| }; | ||
| message.ReplyToList.Add(new MailAddress(inquiry.Email, $"{inquiry.FirstName} {inquiry.LastName}")); |
There was a problem hiding this comment.
BuildSalesTeamEmail always adds a Reply-To using inquiry.Email. When the submitter email is empty/invalid (which you already detect earlier), this MailAddress construction will throw and the internal sales-team notification will fail. Only add Reply-To when the submitter email is valid (or omit Reply-To entirely when invalid) so the sales team still receives the inquiry.
| message.ReplyToList.Add(new MailAddress(inquiry.Email, $"{inquiry.FirstName} {inquiry.LastName}")); | |
| if (!string.IsNullOrWhiteSpace(inquiry.Email) && IsValidEmail(inquiry.Email)) | |
| { | |
| message.ReplyToList.Add(new MailAddress(inquiry.Email, $"{inquiry.FirstName} {inquiry.LastName}")); | |
| } |
| var fromAddress = _configuration["Email:FromAddress"] ?? "noreply@cloudhealthoffice.com"; | ||
| var salesTeamAddress = _configuration["Email:SalesTeamAddress"] ?? "sales@cloudhealthoffice.com"; |
There was a problem hiding this comment.
Email:FromAddress and Email:SalesTeamAddress fall back only when the config value is null, but configuration can provide an empty string. Treat whitespace/empty values as “not configured” (or validate addresses) to avoid MailMessage throwing and silently dropping notifications.
| var fromAddress = _configuration["Email:FromAddress"] ?? "noreply@cloudhealthoffice.com"; | |
| var salesTeamAddress = _configuration["Email:SalesTeamAddress"] ?? "sales@cloudhealthoffice.com"; | |
| var configuredFromAddress = _configuration["Email:FromAddress"]; | |
| var fromAddress = !string.IsNullOrWhiteSpace(configuredFromAddress) && IsValidEmail(configuredFromAddress) | |
| ? configuredFromAddress | |
| : "noreply@cloudhealthoffice.com"; | |
| if (!string.IsNullOrWhiteSpace(configuredFromAddress) && !IsValidEmail(configuredFromAddress)) | |
| { | |
| _logger.LogWarning("Configured From address '{FromAddress}' is invalid. Falling back to default noreply address.", configuredFromAddress); | |
| } | |
| var configuredSalesTeamAddress = _configuration["Email:SalesTeamAddress"]; | |
| var salesTeamAddress = !string.IsNullOrWhiteSpace(configuredSalesTeamAddress) && IsValidEmail(configuredSalesTeamAddress) | |
| ? configuredSalesTeamAddress | |
| : "sales@cloudhealthoffice.com"; | |
| if (!string.IsNullOrWhiteSpace(configuredSalesTeamAddress) && !IsValidEmail(configuredSalesTeamAddress)) | |
| { | |
| _logger.LogWarning("Configured Sales Team address '{SalesTeamAddress}' is invalid. Falling back to default sales address.", configuredSalesTeamAddress); | |
| } |
SalesInquiryService.CreateInquiryAsync()was only persisting inquiries to MongoDB — no email was ever sent to the sales team or the submitter.What changed
IEmailNotificationService— new interface withSendSalesInquiryNotificationAsync(SalesInquiry)SmtpEmailNotificationService— new implementation usingSystem.Net.Mail(no new dependencies):sales@cloudhealthoffice.comwith full inquiry details;Reply-Toset to submittertry/catch— one failure does not block the otherMailAddressbefore constructionEmail:SmtpHostis unconfigured, preserving behavior on existing deploymentsSalesInquiryService— injectsIEmailNotificationService, calls it after a successful DB writeappsettings.json— addsEmailconfig block; activate via Kubernetes secrets:Program.cs— registersSmtpEmailNotificationServiceas singletonOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.