Feature - Add batch email support to .NET client #158
Conversation
…ail request to reuse it in bulk email feature.
- Updated Batch Email Send Request and Response structures - Updated Send Email Response structure - Updated/Corrected validation rules for EmailRequest, SendEmailRequest and BatchEmailRequest and corresponding tests are added/modified - Added tests for BatchEmailResponse and BatchSendEmailResponse to ensure correct serialization and deserialization. - Removed obsolete EmailResponseTests and consolidated error handling in response tests.
…/mailtrap-dotnet into feature/95-batch-email
- Refactor email request validation and documentation improvements
- Added Batch Email Send Example - Updated Response Models: error list no need to be nullable
- documentation improvements - code cleanup for tests
- Few comments added in batch email sending example
WalkthroughRefactors email client APIs to generic types and splits send vs batch clients; adds batch request/response models, builders, and validators; updates endpoint resolution and factory methods to support batch paths; updates DI, examples, docs, and extensive tests; bumps C# LangVersion to 13.0. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant MailtrapClient
participant EmailClientFactory
participant EndpointProvider
participant SendClient as ISendEmailClient
participant API as Mailtrap API
App->>MailtrapClient: Transactional()/Bulk()/Test(inboxId)
MailtrapClient->>EmailClientFactory: Create(isBulk?, inboxId?)
EmailClientFactory->>EndpointProvider: GetRequestUri(isBatch=false,isBulk,inboxId)
EndpointProvider-->>EmailClientFactory: URI (…/send)
EmailClientFactory-->>MailtrapClient: ISendEmailClient
App->>SendClient: Send(SendEmailRequest)
SendClient->>API: POST /send payload
API-->>SendClient: SendEmailResponse
SendClient-->>App: SendEmailResponse
sequenceDiagram
autonumber
actor App
participant MailtrapClient
participant EmailClientFactory
participant EndpointProvider
participant BatchClient as IBatchEmailClient
participant API as Mailtrap API
App->>MailtrapClient: BatchTransactional()/BatchBulk()/BatchTest(inboxId)
MailtrapClient->>EmailClientFactory: CreateBatch(isBulk?, inboxId?)
EmailClientFactory->>EndpointProvider: GetRequestUri(isBatch=true,isBulk,inboxId)
EndpointProvider-->>EmailClientFactory: URI (…/batch)
EmailClientFactory-->>MailtrapClient: IBatchEmailClient
App->>BatchClient: Send(BatchEmailRequest)
BatchClient->>API: POST /batch payload
API-->>BatchClient: BatchEmailResponse
BatchClient-->>App: BatchEmailResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/Mailtrap.Abstractions/TestingMessages/Requests/ForwardTestingMessageRequestValidator.cs (1)
4-13: Add XML documentation to align with established validator patterns.This validator is missing XML documentation. According to the established pattern in this repository, all validators should have comprehensive XML documentation covering the class purpose and the static Instance property.
Based on learnings.
Apply this diff to add XML documentation:
+/// <summary> +/// Validator for <see cref="ForwardTestingMessageRequest"/>. +/// Ensures that the email address is valid. +/// </summary> internal sealed class ForwardTestingMessageRequestValidator : AbstractValidator<ForwardTestingMessageRequest> { + /// <summary> + /// Gets the singleton instance of the validator. + /// </summary> public static ForwardTestingMessageRequestValidator Instance { get; } = new();tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs (1)
93-101: Fix the incorrect test method name.The method is named
Validate_Should_Fail_WhenRequestIsValidbut the test body asserts that validation should pass (result.IsValid.Should().BeTrue()). This is a critical naming inconsistency.Apply this diff to correct the method name:
- public void Validate_Should_Fail_WhenRequestIsValid() + public void Validate_Should_Pass_WhenRequestIsValid() { var request = CreateValidRequest(); var result = request.Validate(); result.IsValid.Should().BeTrue(); result.Errors.Should().BeEmpty(); }src/Mailtrap.Abstractions/IMailtrapClient.cs (1)
49-82: Document breaking return type change and update examples
- Changing
IMailtrapClient.Email(),Transactional(),Bulk(), andTest()return types fromIEmailClient<…>toISendEmailClientis a binary-breaking change. Add a release-note entry and bump the major version.- Update
src/Mailtrap.Abstractions/README.mdanddocs/cookbook/send-email.mdto replaceIEmailClientwithISendEmailClient(andIBatchEmailClientwhere applicable), and include migration snippets.
🧹 Nitpick comments (43)
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs (3)
30-60: Address the ignored test or remove it.The test is ignored due to "Flaky JSON comparison" and contains a TODO comment about finding a more stable assertion approach. An ignored test provides no value and creates maintenance burden.
Consider one of these approaches:
- Remove the test entirely if the round-trip test at lines 18-27 provides sufficient coverage.
- Fix the flakiness by using
JsonDocumentto parse and compare JSON structures rather than string comparison:- [Test] - [Ignore("Flaky JSON comparison")] - public void Should_SerializeAndDeserializeCorrectly_2() + [Test] + public void Should_SerializeTemplateVariablesCorrectly() { var request = SendEmailRequest .Create() .From("[email protected]", "John Doe") .To("[email protected]") .Template("ID") .TemplateVariables(new { Var1 = "First Name", Var2 = "Last Name" }); var serialized = JsonSerializer.Serialize(request, MailtrapJsonSerializerOptions.NotIndented); - - // TODO: Find more stable way to assert JSON serialization. - serialized.Should().Be( - "{" + - "\"from\":{\"email\":\"[email protected]\",\"name\":\"John Doe\"}," + - "\"to\":[{\"email\":\"[email protected]\"}]," + - "\"cc\":[]," + - "\"bcc\":[]," + - "\"attachments\":[]," + - "\"headers\":{}," + - "\"custom_variables\":{}," + - "\"template_uuid\":\"ID\"," + - "\"template_variables\":{\"var1\":\"First Name\",\"var2\":\"Last Name\"}" + - "}"); - - - // Below would not work, considering weakly-typed nature of the template variables property. - //var deserialized = JsonSerializer.Deserialize<TemplatedEmailRequest>(serialized, MailtrapJsonSerializerOptions.NotIndented); - //deserialized.Should().BeEquivalentTo(request); + + using var doc = JsonDocument.Parse(serialized); + var root = doc.RootElement; + + root.GetProperty("from").GetProperty("email").GetString().Should().Be("[email protected]"); + root.GetProperty("template_uuid").GetString().Should().Be("ID"); + root.GetProperty("template_variables").GetProperty("var1").GetString().Should().Be("First Name"); + root.GetProperty("template_variables").GetProperty("var2").GetString().Should().Be("Last Name"); }
79-90: Strengthen the error message assertion.The assertion
.Contain(e => e.Contains("recipient"))is too weak and could pass with unrelated error messages containing the word "recipient".Apply this diff to use a more specific assertion:
var result = req.Validate(); result.IsValid.Should().BeFalse(); - result.Errors.Should().Contain(e => e.Contains("recipient")); + result.Errors.Should().Contain(e => e.Contains("At least one recipient"));Alternatively, if you know the exact error message from the validator, assert on that specific message.
117-135: Strengthen the error message assertion.Similar to the earlier test, the assertion
.Contain(e => e.Contains(invalidRecipientType))is too weak. It could pass if any error message contains "To", "Cc", or "Bcc" in any context.Apply this diff to use a more specific assertion pattern:
var result = request.Validate(); result.IsValid.Should().BeFalse(); - result.Errors.Should().Contain(e => e.Contains(invalidRecipientType)); + result.Errors.Should().Contain(e => + e.Contains(invalidRecipientType) && + (e.Contains("must not exceed") || e.Contains("1000")));This ensures the error is specifically about the recipient count limit, not just any error mentioning the recipient type.
src/Mailtrap/Emails/Requests/BatchEmailRequestBuilder.cs (1)
37-44: Initialize Requests with ??= to avoid throws; avoid depending on caller to pre-initCurrent guards will throw if Requests is null. Safer to coalesce to a concrete list before add.
Apply:
@@ - Ensure.NotNull(batchRequest, nameof(batchRequest)); - Ensure.NotNull(batchRequest.Requests, nameof(batchRequest.Requests)); - Ensure.NotNull(requests, nameof(requests)); + Ensure.NotNull(batchRequest, nameof(batchRequest)); + Ensure.NotNull(requests, nameof(requests)); + batchRequest.Requests ??= new List<SendEmailRequest>(); @@ - batchRequest.Requests.AddRange(requests); + batchRequest.Requests.AddRange(requests); @@ - Ensure.NotNull(batchRequest, nameof(batchRequest)); - Ensure.NotNull(batchRequest.Requests, nameof(batchRequest.Requests)); - Ensure.NotNull(requests, nameof(requests)); + Ensure.NotNull(batchRequest, nameof(batchRequest)); + Ensure.NotNull(requests, nameof(requests)); + batchRequest.Requests ??= new List<SendEmailRequest>(); @@ - batchRequest.Requests.AddRange(requests); + batchRequest.Requests.AddRange(requests); @@ - Ensure.NotNull(batchRequest, nameof(batchRequest)); - Ensure.NotNull(batchRequest.Requests, nameof(batchRequest.Requests)); - Ensure.NotNull(request, nameof(request)); + Ensure.NotNull(batchRequest, nameof(batchRequest)); + Ensure.NotNull(request, nameof(request)); + batchRequest.Requests ??= new List<SendEmailRequest>();Note: if List requires a using, fully qualify or ensure System.Collections.Generic is available.
Also applies to: 72-79, 107-114
src/Mailtrap/Emails/Requests/EmailRequestBuilder.cs (4)
195-204: Make Attach resilient: coalesce Attachments before AddRangeAvoid throwing if Attachments is null by initializing it.
public static T Attach<T>(this T request, params Attachment[] attachments) where T : EmailRequest { Ensure.NotNull(request, nameof(request)); - Ensure.NotNull(request.Attachments, nameof(request.Attachments)); Ensure.NotNull(attachments, nameof(attachments)); + request.Attachments ??= new List<Attachment>(); request.Attachments.AddRange(attachments); return request; }
333-342: Initialize Headers to avoid null dereference; key guard is goodCoalesce the dictionary before use.
public static T Header<T>(this T request, string key, string value) where T : EmailRequest { Ensure.NotNull(request, nameof(request)); - Ensure.NotNull(request.Headers, nameof(request.Headers)); Ensure.NotNullOrEmpty(key, nameof(key)); + request.Headers ??= new Dictionary<string, string>(); request.Headers[key] = value; return request; }
416-425: Initialize CustomVariables to avoid null dereference; key guard is goodCoalesce the dictionary before use.
public static T CustomVariable<T>(this T request, string key, string value) where T : EmailRequest { Ensure.NotNull(request, nameof(request)); - Ensure.NotNull(request.CustomVariables, nameof(request.CustomVariables)); Ensure.NotNullOrEmpty(key, nameof(key)); + request.CustomVariables ??= new Dictionary<string, string>(); request.CustomVariables[key] = value; return request; }
633-641: Optional: clear conflicting fields when TemplateId is set (per remarks)Docs state Subject, TextBody, HtmlBody, and Category are forbidden when TemplateId is used. Builder can enforce this for users.
public static T Template<T>(this T request, string templateId) where T : EmailRequest { Ensure.NotNull(request, nameof(request)); Ensure.NotNullOrEmpty(templateId, nameof(templateId)); request.TemplateId = templateId; + // Clear fields that must be null when using templates + request.Subject = null; + request.TextBody = null; + request.HtmlBody = null; + request.Category = null; return request; }src/Mailtrap.Abstractions/Emails/Models/AttachmentValidator.cs (1)
4-22: Add XML documentation to match validator conventions.The validator implementation looks good, but we lost the XML summary that accompanies every validator in this codebase. Please add class-level (and
Instanceproperty) XML docs so the generated API documentation stays consistent. Based on learningssrc/Mailtrap/Emails/EmailClientEndpointProvider.cs (1)
13-22: Refresh the XML docs to match the method behavior.This comment block still describes a constructor and misrepresents the parameter semantics. Please rewrite it so the summary and
<param>tags explain the URI-building logic (batch vs send, bulk host selection, inbox scoping).- /// <summary> - /// Initializes a new instance of the <see cref="EmailClientEndpointProvider"/> class. - /// </summary> - /// <param name="isBatch">if <c>true</c> the batch segment will be used; otherwise, the regular send segment will be used.</param> - /// <param name="isBulk">if <c>true</c> the bulk email endpoint will be used; - /// if <c>true</c> and <paramref name="inboxId"/> is provided, the test email endpoint will be used; - /// otherwise, the single email endpoint will be used.</param> - /// <param name="inboxId">If inbox identifier provided, the request will be scoped to that inbox.</param> - /// <returns></returns> + /// <summary> + /// Builds the request <see cref="Uri"/> for send or batch operations. + /// </summary> + /// <param name="isBatch">When <c>true</c>, append the batch segment; otherwise append the send segment.</param> + /// <param name="isBulk">When <c>true</c>, use the bulk host; otherwise use the standard send host.</param> + /// <param name="inboxId">When supplied, scope the request to the inbox and switch to the test host.</param> + /// <returns>The fully qualified endpoint URI.</returns>examples/Mailtrap.Example.Email.BatchSend/Program.cs (2)
96-99: Remove FailFast in example; prefer clean exit/logging
Environment.FailFastterminates the process abruptly and is unsuitable for sample apps. Log and rethrow or set exit code.- Environment.FailFast(ex.Message); - throw; + // Consider returning non-zero exit code instead of fail-fast in examples + Environment.ExitCode = -1; + throw;
291-304: Guard file I/O to avoid crashes when example files are missing
File.ReadAllByteson hardcoded path can throw. Mirror the existence check used earlier and skip when missing.- var filePath = @"C:\files\preview.pdf"; - var fileName = "preview.pdf"; - var bytes = File.ReadAllBytes(filePath); - var fileContent = Convert.ToBase64String(bytes); - var attachment = new Attachment( - content: fileContent, - fileName: fileName, - disposition: DispositionType.Attachment, - mimeType: MediaTypeNames.Application.Pdf, - contentId: "attachment_1"); - - request.Attachments.Add(attachment); + var filePath = @"C:\files\preview.pdf"; + if (File.Exists(filePath)) + { + var fileName = "preview.pdf"; + var bytes = File.ReadAllBytes(filePath); + var fileContent = Convert.ToBase64String(bytes); + var attachment = new Attachment( + content: fileContent, + fileName: fileName, + disposition: DispositionType.Attachment, + mimeType: MediaTypeNames.Application.Pdf, + contentId: "attachment_1"); + + request.Attachments.Add(attachment); + }src/Mailtrap.Abstractions/Emails/Extensions/BatchEmailRequestExtensions.cs (2)
15-18: Return empty sequence instead of null to simplify consumersAvoid returning null; return an empty enumerable when
Requestsis null. Optionally materialize to avoid deferred enumeration surprises.- internal static IEnumerable<SendEmailRequest>? GetMergedRequests(this BatchEmailRequest batchRequest) + internal static IEnumerable<SendEmailRequest> GetMergedRequests(this BatchEmailRequest batchRequest) { - return batchRequest.Requests?.Select(request => MergeWithBase(request, batchRequest.Base)); + return (batchRequest.Requests ?? Enumerable.Empty<SendEmailRequest>()) + .Select(request => MergeWithBase(request, batchRequest.Base)); }
31-48: Clarify “empty string means inherit” and shallow-copy of collections
- Using
string.IsNullOrEmptytreats empty as “unset,” inheriting from base. If empty should override base with empty, switch torequest.Subject is null ? baseRequest.Subject : request.Subject(same for Text/Html/Category/TemplateId) orIsNullOrWhiteSpaceif whitespace should inherit. Please confirm intended behavior.Attachments/Headers/CustomVariables/TemplateVariablesare shallow-copied references. Merged request may alias base collections, causing accidental cross-mutation. Consider cloning when inheriting.Example adjustment:
- Subject = string.IsNullOrEmpty(request.Subject) ? baseRequest.Subject : request.Subject, + Subject = request.Subject is null ? baseRequest.Subject : request.Subject, ... - Attachments = request.Attachments ?? baseRequest.Attachments, + Attachments = request.Attachments ?? (baseRequest.Attachments is null ? null : new List<Attachment>(baseRequest.Attachments)), - Headers = request.Headers ?? baseRequest.Headers, + Headers = request.Headers ?? (baseRequest.Headers is null ? null : new Dictionary<string,string>(baseRequest.Headers)), - CustomVariables = request.CustomVariables ?? baseRequest.CustomVariables, + CustomVariables = request.CustomVariables ?? (baseRequest.CustomVariables is null ? null : new Dictionary<string,string>(baseRequest.CustomVariables)), - TemplateVariables = request.TemplateVariables ?? baseRequest.TemplateVariables + TemplateVariables = request.TemplateVariables ?? (baseRequest.TemplateVariables is null ? null : new Dictionary<string,string>(baseRequest.TemplateVariables))tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.cs (1)
56-59: Confirm validator messaging vs spec (“at least one of Text/Html”)The test expects both "'Text Body' must not be empty." and "'Html Body' must not be empty." when both are empty. If the rule is “at least one must be specified,” consider a single aggregate message or conditional messages to avoid implying both are required.
tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.HtmlBody.cs (1)
46-46: Use value equality for strings in assertions
BeSameAschecks reference identity and can be brittle with string interning. PreferBe(_html)/Be(otherHtml).- request.HtmlBody.Should().BeSameAs(_html); + request.HtmlBody.Should().Be(_html); ... - request.HtmlBody.Should().BeSameAs(otherHtml); + request.HtmlBody.Should().Be(otherHtml);Also applies to: 59-59
tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.From.cs (1)
63-66: Remove unused local variableThe local request is unused in this test. Drop it to avoid warnings.
- var request = EmailRequest.Create(); - var act = () => EmailRequestBuilder.From<EmailRequest>(null!, SenderEmail);src/Mailtrap.Abstractions/Emails/Validators/EmailRequestValidator.cs (3)
65-78: Use default messages for simple Null rules to match validator styleCustom messages on Null rules are unnecessary per repo convention. Keep custom text only for complex rules.
When(r => !string.IsNullOrEmpty(r.TemplateId), () => { RuleFor(r => r.Subject) - .Null() - .WithMessage($"'{nameof(EmailRequest.Subject)}' should be null, when '{nameof(EmailRequest.TemplateId)}' is specified."); + .Null(); RuleFor(r => r.TextBody) - .Null() - .WithMessage($"'{nameof(EmailRequest.TextBody)}' should be null, when '{nameof(EmailRequest.TemplateId)}' is specified."); + .Null(); RuleFor(r => r.HtmlBody) - .Null() - .WithMessage($"'{nameof(EmailRequest.HtmlBody)}' should be null, when '{nameof(EmailRequest.TemplateId)}' is specified."); + .Null(); });Based on learnings
45-63: Avoid duplicate errors for “either TextBody or HtmlBody” by using a single ruleCurrent dual rules can emit two errors when both are empty/null. Consolidate into one Must with a single message; rely on default messages for Subject.
- When(r => string.IsNullOrEmpty(r.TemplateId), () => - { - RuleFor(r => r.Subject) - .NotNull() - .MinimumLength(1) - .When(r => !isBase); - - RuleFor(r => r.TextBody) - .NotNull() - .MinimumLength(1) - .When(r => string.IsNullOrEmpty(r.HtmlBody) && !isBase) - .WithMessage($"Either '{nameof(EmailRequest.TextBody)}' or '{nameof(EmailRequest.HtmlBody)}' or both should be set to non-empty string, when template is not specified."); - - RuleFor(r => r.HtmlBody) - .NotNull() - .MinimumLength(1) - .When(r => string.IsNullOrEmpty(r.TextBody) && !isBase) - .WithMessage($"Either '{nameof(EmailRequest.TextBody)}' or '{nameof(EmailRequest.HtmlBody)}' or both should be set to non-empty string, when template is not specified."); - }); + When(r => string.IsNullOrEmpty(r.TemplateId) && !isBase, () => + { + RuleFor(r => r.Subject).NotEmpty(); + + RuleFor(r => r) + .Must(r => !string.IsNullOrEmpty(r.TextBody) || !string.IsNullOrEmpty(r.HtmlBody)) + .WithMessage($"Either '{nameof(EmailRequest.TextBody)}' or '{nameof(EmailRequest.HtmlBody)}' or both should be set to non-empty string, when template is not specified."); + });Based on learnings
19-23: Fix XML doc wording for claritySmall grammar tweak.
- /// If <paramref name="isBase"/> is true, the validator is configured for Base request validation. - /// This means that the no properties are required and perform basic validation. + /// If <paramref name="isBase"/> is true, the validator is configured for Base request validation. + /// This means no properties are required; only basic validations are performed.src/Mailtrap.Abstractions/Emails/Requests/BatchEmailRequest.cs (5)
10-12: XML doc typo“Gets or sets and object” → “Gets or sets an object.” Minor fix.
- /// Gets or sets and object with general properties of all emails in the batch.<br /> + /// Gets or sets an object with general properties of all emails in the batch.<br />
17-20: Make Base optional to avoid serializing empty objects and unnecessary validationDefaulting Base to new() causes “base”: {} to be serialized and always validated. Prefer nullable Base and omit when null.
- [JsonPropertyName("base")] - [JsonPropertyOrder(1)] - public EmailRequest Base { get; set; } = new(); + [JsonPropertyName("base")] + [JsonPropertyOrder(1)] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public EmailRequest? Base { get; set; }Note: Merge/validator code already accepts EmailRequest? in MergeWithBase; builders continue to set Base explicitly.
35-37: Preserve list instance on deserializationAlign with Errors in responses and avoid replacement on deserialize by populating the existing list.
[JsonPropertyName("requests")] [JsonPropertyOrder(2)] - public IList<SendEmailRequest> Requests { get; set; } = []; + [JsonObjectCreationHandling(JsonObjectCreationHandling.Populate)] + public IList<SendEmailRequest> Requests { get; set; } = [];
31-34: 50 MB remark not enforced in validatorDocs mention 50 MB total payload; current validator enforces only count<=500. Decide if the client should pre-validate size or leave to server. I can add a conservative preflight check (sum of base+items attachments), if desired.
49-54: Validator message style consistencyBatchEmailRequestValidator uses custom WithMessage for simple rules; project style favors default FluentValidation messages for such cases.
Based on learnings
src/Mailtrap.Abstractions/Emails/Responses/BatchSendEmailResponse.cs (2)
9-12: XML doc grammar“Gets errors, associated with the response.” → “Gets the errors associated with the response.”
- /// Gets errors, associated with the response. + /// Gets the errors associated with the response.
22-31: Prefer direct assignment for MessageIds (consistent with SendEmailResponse)Simplify CreateSuccess/Failure overload and match base pattern.
internal static new BatchSendEmailResponse CreateSuccess(params string[] messageIds) { - var response = new BatchSendEmailResponse - { - Success = true, - }; - messageIds.ToList().ForEach(response.MessageIds.Add); - - return response; + return new BatchSendEmailResponse + { + Success = true, + MessageIds = messageIds + }; } @@ internal static BatchSendEmailResponse CreateFailure(string[] messageIds, string[] errors) { - var response = new BatchSendEmailResponse - { - Success = false, - Errors = errors - }; - messageIds.ToList().ForEach(response.MessageIds.Add); - return response; + return new BatchSendEmailResponse + { + Success = false, + Errors = errors, + MessageIds = messageIds + }; }Also applies to: 42-51
tests/Mailtrap.UnitTests/Emails/EmailClientFactoryTests.cs (1)
76-76: Assert interfaces, not concrete implementationsReduce coupling to implementation details by asserting assignability to ISendEmailClient/IBatchEmailClient.
- result.Should().BeOfType<SendEmailClient>(); + result.Should().BeAssignableTo<ISendEmailClient>(); @@ - result.Should().BeOfType<SendEmailClient>(); + result.Should().BeAssignableTo<ISendEmailClient>(); @@ - result.Should().BeOfType<SendEmailClient>(); + result.Should().BeAssignableTo<ISendEmailClient>(); @@ - result.Should().BeOfType<SendEmailClient>(); + result.Should().BeAssignableTo<ISendEmailClient>(); @@ - result.Should().BeOfType<SendEmailClient>(); + result.Should().BeAssignableTo<ISendEmailClient>(); @@ - result.Should().BeOfType<BatchEmailClient>(); + result.Should().BeAssignableTo<IBatchEmailClient>(); @@ - result.Should().BeOfType<BatchEmailClient>(); + result.Should().BeAssignableTo<IBatchEmailClient>(); @@ - result.Should().BeOfType<BatchEmailClient>(); + result.Should().BeAssignableTo<IBatchEmailClient>(); @@ - result.Should().BeOfType<BatchEmailClient>(); + result.Should().BeAssignableTo<IBatchEmailClient>(); @@ - result.Should().BeOfType<BatchEmailClient>(); + result.Should().BeAssignableTo<IBatchEmailClient>();Optional: add Verify on endpoint provider mocks to ensure correct URI selection per case.
Also applies to: 102-102, 128-128, 154-154, 180-181, 207-207, 233-233, 259-259, 285-285, 311-311
tests/Mailtrap.UnitTests/Emails/Models/AttachmentValidatorTests.cs (1)
272-275: Use valid emails in length-only tests to avoid unrelated failuresGenerate syntactically valid addresses when testing only collection size.
- for (var i = 1; i <= 1001; i++) + for (var i = 1; i <= 1001; i++) { - internalRequest.Cc($"recipient{i}.domain.com"); + internalRequest.Cc($"recipient{i}@domain.com"); } @@ - for (var i = 1; i <= 1000; i++) + for (var i = 1; i <= 1000; i++) { - internalRequest.Cc($"recipient{i}.domain.com"); + internalRequest.Cc($"recipient{i}@domain.com"); } @@ - for (var i = 1; i <= 1000; i++) + for (var i = 1; i <= 1000; i++) { - internalRequest.Bcc($"recipient{i}.domain.com"); + internalRequest.Bcc($"recipient{i}@domain.com"); }Also applies to: 289-292, 350-353
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (1)
90-99: Add a robustness test for null items in RequestsEnsure a null element in Requests doesn’t throw and yields a validation error at the correct path.
+ [Test] + public void Validation_Requests_Should_Fail_WhenItemIsNull() + { + var request = BatchEmailRequest.Create(); + request.Requests.Add(null!); + + var result = BatchEmailRequestValidator.Instance.TestValidate(request); + + result.ShouldHaveValidationErrorFor($"{nameof(BatchEmailRequest.Requests)}[0]"); + }tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.Attachment.cs (1)
78-89: Consider making the helper more flexible.The
Attach_CreateAndValidatehelper method acceptsparams Attachment[] attachmentsbut hardcodes an expectation of exactly 2 attachments at line 85. While this works for the current usage, it reduces reusability.Consider updating the assertion to be more flexible:
-private static EmailRequest Attach_CreateAndValidate(params Attachment[] attachments) +private EmailRequest Attach_CreateAndValidate(params Attachment[] attachments) { var request = EmailRequest .Create() .Attach(attachments); request.Attachments.Should() - .HaveCount(2).And + .HaveCount(attachments.Length).And .ContainInOrder(attachments); return request; }tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestTests.Validator.cs (1)
7-9: Consider using readonly fields for test data.The test data fields are never reassigned, so they could be declared as readonly fields instead of properties for clarity and slight performance improvement.
-private string _validEmail { get; } = "[email protected]"; -private string _invalidEmail { get; } = "someone"; -private string _templateId { get; } = "ID"; +private readonly string _validEmail = "[email protected]"; +private readonly string _invalidEmail = "someone"; +private readonly string _templateId = "ID";src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (1)
96-99: Ensure Attachments is a mutable collection
= []may materialize as an array depending on language version/target and can be fixed-size at runtime. Use a List to guarantee mutability for builder extensions and validators.- public IList<Attachment> Attachments { get; set; } = []; + public IList<Attachment> Attachments { get; set; } = new List<Attachment>();tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.Category.cs (1)
45-46: Prefer value equality for string assertionsUse
Beinstead ofBeSameAsto avoid relying on string reference identity.- request.Category.Should().BeSameAs(_category); + request.Category.Should().Be(_category); ... - request.Category.Should().BeSameAs(otherCategory); + request.Category.Should().Be(otherCategory);Also applies to: 58-59
tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.Subject.cs (1)
45-46: Prefer value equality for string assertionsUse
Beto assert string content rather than reference identity.- request.Subject.Should().BeSameAs(_subject); + request.Subject.Should().Be(_subject); ... - request.Subject.Should().BeSameAs(otherSubject); + request.Subject.Should().Be(otherSubject);Also applies to: 58-59
src/Mailtrap.Abstractions/Emails/Responses/SendEmailResponse.cs (2)
29-33: Guarantee a mutable MessageIds collection
= []can yield a fixed-size collection. Prefer an explicitList<string>to ensure consistent mutability and serializer population semantics.- [JsonObjectCreationHandling(JsonObjectCreationHandling.Populate)] - public IList<string> MessageIds { get; private set; } = []; + [JsonObjectCreationHandling(JsonObjectCreationHandling.Populate)] + public IList<string> MessageIds { get; private set; } = new List<string>();
35-42: Avoid assigning arrays to MessageIds; populate the list insteadAssigning
string[]toIList<string>can produce a fixed-size list; subsequent additions would throw. Populate the existing list (mirrors BatchSendEmailResponse.CreateSuccess).-internal static SendEmailResponse CreateSuccess(params string[] messageIds) -{ - return new SendEmailResponse - { - Success = true, - MessageIds = messageIds - }; -} +internal static SendEmailResponse CreateSuccess(params string[] messageIds) +{ + var response = new SendEmailResponse + { + Success = true, + }; + if (messageIds is { Length: > 0 }) + { + foreach (var id in messageIds) + { + response.MessageIds.Add(id); + } + } + return response; +}src/Mailtrap/MailtrapClient.cs (1)
12-13: Eager init of default clients; consider lazy for startup perfCreating defaults in ctor is fine. Optionally defer creation until first use to reduce startup cost.
Also applies to: 29-31
tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.ReplyTo.cs (1)
61-66: Remove unused local variableThe local variable ‘request’ is unused in this test. Safe to remove.
tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.CustomVariable.cs (1)
64-67: Avoid relying on dictionary enumeration order in assertionsUsing ContainInOrder on a dictionary can be brittle. Prefer key/value presence and counts (ContainKeys, BeEquivalentTo) without ordering.
Also applies to: 78-81, 90-93
src/Mailtrap.Abstractions/Emails/Validators/SendEmailRequestValidator.cs (1)
4-8: Clarify XML doc to mention composed validations (optional).
Add that this validator also applies EmailRequestValidator rules (From/ReplyTo/attachments/body/template), not only recipients.tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Base.cs (1)
28-35: Tests are comprehensive and aligned with base-mode validation.
Optional: use EmailRequest.Create() consistently when passing to Base(...) for readability (SendEmailRequest works via inheritance).src/Mailtrap.Abstractions/Emails/Validators/BatchEmailRequestValidator.cs (1)
20-26: Prefer default FluentValidation messages for simple rules (consistency).
Remove custom WithMessage for NotEmpty and LessThanOrEqualTo to align with validator conventions.Based on learnings
- RuleFor(r => r.Requests) - .NotEmpty() - .WithMessage("'Requests' must not be empty."); + RuleFor(r => r.Requests) + .NotEmpty(); - RuleFor(r => r.Requests.Count) - .LessThanOrEqualTo(500) - .WithMessage("'Requests' shouldn't exceed 500 records.") - .When(r => r.Requests is not null); + RuleFor(r => r.Requests.Count) + .LessThanOrEqualTo(500) + .When(r => r.Requests is not null);
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs
Show resolved
Hide resolved
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs
Show resolved
Hide resolved
dr-3lo
changed the title
- documentation and enhance fluent builder methods
- Update error handling in email sending to set exit code instead of failing fast. - Improve attachment MIME type detection based on file extension. - Add null check for SendEmailRequest in MergeWithBase method. - Update documentation for BatchEmailRequest and EmailRequest to clarify properties. - Refactor tests to ensure proper validation and serialization of email requests.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/cookbook/send-email.md (1)
300-303: Typo: use OperationCanceledException (single L).Update the catch clause.
Apply this diff:
-catch (OperationCancelledException ocex) +catch (OperationCanceledException ocex) { // handle cancellation }
♻️ Duplicate comments (2)
src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (1)
161-166: Doc/validation mismatch: Category with TemplateIdDocs say Category must be null when TemplateId is set, but the validator allows Category (only enforces length). Update docs to reflect behavior.
- /// If provided, then <see cref="Subject"/>, <see cref="Category"/>, <see cref="TextBody"/> and <see cref="HtmlBody"/> - /// properties are forbidden and must be <see langword="null"/>.<br /> - /// Email subject, text and html will be generated from template using optional <see cref="TemplateVariables"/>. + /// If provided, then <see cref="Subject"/>, <see cref="TextBody"/> and <see cref="HtmlBody"/> + /// are forbidden and must be <see langword="null"/>.<br /> + /// Email subject, text and html will be generated from the template using optional <see cref="TemplateVariables"/>.tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs (1)
204-207: Thank you for fixing the test email formats in CC/BCC loopsThe updated addresses now include '@', so positive-path tests won’t fail on format. LGTM.
Also applies to: 219-222, 268-271, 283-286
🧹 Nitpick comments (15)
tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.ReplyTo.cs (1)
133-133: Consider a more descriptive test name.The
_2suffix differentiates this test from the one at line 43, but the distinction could be clearer. Consider renaming to something likeReplyTo_Should_OverrideReplyTo_WhenCalledSeveralTimesWithStringOverloadto explicitly indicate it tests the string parameter overload.tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (2)
39-39: Simplify test parameter.The
[Values]attribute with a single value is unnecessary here. Consider using[Test]without parameters or inline the value directly in the test body for clarity.Apply this diff:
- public void Validation_Should_Fail_WhenRequestsCountIsGreaterThan500([Values(501)] int count) + public void Validation_Should_Fail_WhenRequestsCountIsGreaterThan500() { + const int count = 501; var request = BatchEmailRequest.Create()
221-221: Simplify test parameter.The
[Values]attribute with a single value is unnecessary. Consider removing the parameter and using a const value directly.Apply this diff:
- public void Validation_Requests_Should_Fail_WhenToLengthExceedsLimit([Values(1001)] int count) + public void Validation_Requests_Should_Fail_WhenToLengthExceedsLimit() { + const int count = 1001; var request = BatchEmailRequest.Create()src/Mailtrap.Abstractions/README.md (1)
11-12: Call out the breaking change and add a 1–2 line migration note.Since IEmailClient was replaced by ISendEmailClient and IBatchEmailClient, add a short “Breaking changes” note or a parenthetical in Main Types to guide users (e.g., “IEmailClient → ISendEmailClient/IBatchEmailClient; see docs for migration”). This reduces upgrade friction.
docs/cookbook/batch-send-email.md (2)
294-309: Make the link text descriptive (MD059).Apply this diff:
-## See also -More examples available [here](https://github.com/railsware/mailtrap-dotnet/tree/main/examples). +## See also +More examples: [Mailtrap .NET examples on GitHub](https://github.com/railsware/mailtrap-dotnet/tree/main/examples).
80-98: Optional: add missing import in snippets using MediaTypeNames/DispositionType.Readers may need using System.Net.Mime; to compile examples as-is.
Consider adding:
using System.Net.Mime;Also applies to: 160-174
docs/cookbook/send-email.md (1)
339-340: Make the link text descriptive (MD059).Apply this diff:
-## See also -More examples available [here](https://github.com/railsware/mailtrap-dotnet/tree/main/examples). +## See also +More examples: [Mailtrap .NET examples on GitHub](https://github.com/railsware/mailtrap-dotnet/tree/main/examples).src/Mailtrap/Emails/EmailClientEndpointProvider.cs (1)
20-20: Avoid boolean-flag API; consider enum or dedicated methods.Two booleans are easy to misorder/misuse. Prefer:
- GetSendRequestUri(isBulk, inboxId) and GetBatchRequestUri(isBulk, inboxId), or
- GetRequestUri(EmailOperation.Send|Batch, isBulk, inboxId).
Improves readability and call-site safety.
tests/Mailtrap.UnitTests/Emails/EmailClientFactoryTests.cs (2)
186-209: Solid coverage for batch factory paths. Minor nits: add Verify, rename var.
- Add Moq Verify to assert correct args were used.
- Rename sendUri → batchUri in batch tests for clarity.
Example for one test:
- var sendUri = new Uri("https://localhost/api/batch"); + var batchUri = new Uri("https://localhost/api/batch"); ... - .Returns(sendUri); + .Returns(batchUri); ... - result.ResourceUri.Should().Be(sendUri); + result.ResourceUri.Should().Be(batchUri); + emailClientEndpointProviderMock.Verify( + x => x.GetRequestUri(true, isBulk, inboxId), + Times.Once);Also applies to: 212-235, 238-261, 264-287, 290-313
55-55: Use positive inboxId values in test data.[Random(2)] for long can yield 0/negative, which isn’t a valid inbox id. Prefer explicit positives:
-[Test] -public void Create_ShouldReturnEmailClient([Values] bool isBulk, [Random(2)] long inboxId) +[Test] +public void Create_ShouldReturnEmailClient([Values] bool isBulk, [Values(1L, 42L)] long inboxId)Apply similarly to other tests.
src/Mailtrap/Emails/Requests/BatchEmailRequestBuilder.cs (2)
41-41: EnsureAddRangeextension is in scope and accessibleThese calls rely on
CollectionExtensions.AddRange<T>:
- Import its namespace explicitly, or the extension won’t resolve without a global using.
- If
AddRangeisinternalin another assembly, ensureInternalsVisibleToor make itpublic; otherwise this won’t compile.Recommend adding an explicit import to avoid relying on globals:
+using Mailtrap.Core.Extensions; namespace Mailtrap.Emails.Requests;Also applies to: 76-76
28-35: Fix XML docs: remove non-standardidattribute on<exception>C# XML docs expect
cref(notid). Dropid="ArgumentNullException"to avoid analyzer/doc tooling warnings.tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.Attachment.cs (1)
100-105: Remove unused local to reduce noise
requestis declared but unused in this test. Safe to remove.- var request = EmailRequest.Create(); - var act = () => EmailRequestBuilder.Attach<EmailRequest>(null!, Content, FileName);tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs (1)
41-66: Strengthen JSON shape assertions (add from.name, keep fixture fidelity)Consider also asserting the sender display name to fully cover the "from" shape:
- Check from.name == "John Doe".
This keeps the test aligned with the server’s expected payload shape. Based on learnings.
- root.GetProperty("from").GetProperty("email").GetString().Should().Be("[email protected]"); + var from = root.GetProperty("from"); + from.GetProperty("email").GetString().Should().Be("[email protected]"); + from.GetProperty("name").GetString().Should().Be("John Doe");tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs (1)
26-35: Target the ‘Recipients’ rule explicitly for clarityThese tests intend to validate the recipients rule only. Asserting on the root object (r => r) can pass even when other property-level errors exist (From/Subject/Body). Prefer asserting the specific path to make intent explicit and resilient.
- result.ShouldNotHaveValidationErrorFor(r => r); + result.ShouldNotHaveValidationErrorFor("Recipients");Apply similarly to “OnlyCcRecipientsPresent” and “OnlyBccRecipientsPresent”.
Also applies to: 38-47, 50-59
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
docs/cookbook/batch-send-email.md(1 hunks)docs/cookbook/send-email.md(1 hunks)examples/Mailtrap.Example.Email.BatchSend/Program.cs(1 hunks)src/Mailtrap.Abstractions/Emails/Extensions/BatchEmailRequestExtensions.cs(1 hunks)src/Mailtrap.Abstractions/Emails/Requests/BatchEmailRequest.cs(1 hunks)src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs(1 hunks)src/Mailtrap.Abstractions/Emails/Responses/BatchSendEmailResponse.cs(1 hunks)src/Mailtrap.Abstractions/Emails/Validators/SendEmailRequestValidator.cs(1 hunks)src/Mailtrap.Abstractions/README.md(1 hunks)src/Mailtrap/Emails/EmailClientEndpointProvider.cs(1 hunks)src/Mailtrap/Emails/Requests/BatchEmailRequestBuilder.cs(1 hunks)tests/Mailtrap.UnitTests/Emails/EmailClientFactoryTests.cs(10 hunks)tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Base.cs(1 hunks)tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs(1 hunks)tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.Attachment.cs(6 hunks)tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.From.cs(3 hunks)tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.ReplyTo.cs(3 hunks)tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs(35 hunks)tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Mailtrap.Abstractions/Emails/Requests/BatchEmailRequest.cs
- tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Base.cs
- src/Mailtrap.Abstractions/Emails/Responses/BatchSendEmailResponse.cs
- src/Mailtrap.Abstractions/Emails/Extensions/BatchEmailRequestExtensions.cs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-04T12:23:59.276Z
Learnt from: dr-3lo
PR: railsware/mailtrap-dotnet#139
File: tests/Mailtrap.IntegrationTests/Contacts/Update_Success.json:16-17
Timestamp: 2025-09-04T12:23:59.276Z
Learning: Test fixtures in the Mailtrap .NET client should accurately represent the actual server response format and should not be modified to match client-side converters or serialization preferences.
Applied to files:
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs
🧬 Code graph analysis (12)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (6)
src/Mailtrap.Abstractions/Emails/Requests/BatchEmailRequest.cs (1)
BatchEmailRequest(47-47)src/Mailtrap/Emails/Requests/BatchEmailRequestBuilder.cs (6)
BatchEmailRequest(35-44)BatchEmailRequest(70-79)BatchEmailRequest(105-114)BatchEmailRequest(141-151)BatchEmailRequest(177-186)BatchEmailRequest(208-216)src/Mailtrap.Abstractions/Emails/Validators/BatchEmailRequestValidator.cs (2)
BatchEmailRequestValidator(9-34)BatchEmailRequestValidator(13-33)src/Mailtrap.Abstractions/Emails/Extensions/BatchEmailRequestExtensions.cs (1)
SendEmailRequest(31-54)src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (1)
EmailRequest(194-194)src/Mailtrap.Abstractions/Emails/Models/EmailAddress.cs (1)
EmailAddress(62-68)
src/Mailtrap/Emails/EmailClientEndpointProvider.cs (4)
src/Mailtrap/Emails/IEmailClientEndpointProvider.cs (1)
Uri(9-9)src/Mailtrap/Core/Extensions/UriExtensions.cs (7)
Uri(12-19)Uri(24-31)Uri(36-43)Uri(48-60)Uri(66-72)Uri(79-92)Uri(101-109)src/Mailtrap/Core/Constants/Endpoints.cs (1)
Endpoints(7-32)src/Mailtrap/Core/Constants/UrlSegments.cs (1)
UrlSegments(4-11)
src/Mailtrap.Abstractions/Emails/Validators/SendEmailRequestValidator.cs (2)
src/Mailtrap.Abstractions/Emails/Validators/EmailRequestValidator.cs (2)
EmailRequestValidator(12-80)EmailRequestValidator(23-79)src/Mailtrap.Abstractions/Emails/Validators/SendEmailRecipientsValidator.cs (2)
SendEmailRecipientsValidator(10-42)SendEmailRecipientsValidator(14-41)
tests/Mailtrap.UnitTests/Emails/EmailClientFactoryTests.cs (3)
src/Mailtrap/Emails/EmailClientFactory.cs (12)
ISendEmailClient(29-34)ISendEmailClient(36-36)ISendEmailClient(38-38)ISendEmailClient(40-40)ISendEmailClient(42-42)EmailClientFactory(7-59)EmailClientFactory(14-26)IBatchEmailClient(45-50)IBatchEmailClient(52-52)IBatchEmailClient(54-54)IBatchEmailClient(56-56)IBatchEmailClient(58-58)src/Mailtrap/Emails/EmailClientEndpointProvider.cs (1)
Uri(20-33)src/Mailtrap/Emails/IEmailClientEndpointProvider.cs (1)
Uri(9-9)
tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.From.cs (3)
src/Mailtrap/Emails/Requests/EmailRequestBuilder.cs (1)
EmailRequestBuilder(7-679)src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (1)
EmailRequest(194-194)src/Mailtrap.Abstractions/Emails/Models/EmailAddress.cs (1)
EmailAddress(62-68)
src/Mailtrap/Emails/Requests/BatchEmailRequestBuilder.cs (4)
examples/Mailtrap.Example.Email.BatchSend/Program.cs (8)
BatchEmailRequest(106-136)BatchEmailRequest(142-166)SendEmailRequest(171-191)SendEmailRequest(197-206)SendEmailRequest(212-224)SendEmailRequest(229-267)SendEmailRequest(272-328)SendEmailRequest(333-407)src/Mailtrap.Abstractions/Emails/Requests/BatchEmailRequest.cs (1)
BatchEmailRequest(47-47)src/Mailtrap.Abstractions/Core/Extensions/Ensure.cs (2)
Ensure(9-106)NotNull(18-33)src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (1)
EmailRequest(194-194)
examples/Mailtrap.Example.Email.BatchSend/Program.cs (8)
src/Mailtrap.Abstractions/Emails/IEmailClient.cs (1)
Task(67-67)src/Mailtrap.Abstractions/Emails/Requests/BatchEmailRequest.cs (2)
BatchEmailRequest(47-47)ValidationResult(50-55)src/Mailtrap/Emails/Requests/BatchEmailRequestBuilder.cs (6)
BatchEmailRequest(35-44)BatchEmailRequest(70-79)BatchEmailRequest(105-114)BatchEmailRequest(141-151)BatchEmailRequest(177-186)BatchEmailRequest(208-216)src/Mailtrap.Abstractions/Emails/Responses/BatchEmailResponse.cs (2)
BatchEmailResponse(51-58)BatchEmailResponse(60-67)src/Mailtrap.Abstractions/Emails/Responses/BatchSendEmailResponse.cs (3)
BatchSendEmailResponse(22-31)BatchSendEmailResponse(33-40)BatchSendEmailResponse(42-51)src/Mailtrap.Abstractions/Emails/Models/EmailAddress.cs (1)
EmailAddress(62-68)src/Mailtrap.Abstractions/Emails/Extensions/BatchEmailRequestExtensions.cs (1)
SendEmailRequest(31-54)src/Mailtrap.Abstractions/Emails/Models/Attachment.cs (1)
Attachment(159-174)
tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.ReplyTo.cs (3)
src/Mailtrap/Emails/Requests/EmailRequestBuilder.cs (1)
EmailRequestBuilder(7-679)src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (1)
EmailRequest(194-194)src/Mailtrap.Abstractions/Emails/Models/EmailAddress.cs (1)
EmailAddress(62-68)
src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (3)
src/Mailtrap.Abstractions/Emails/Models/EmailAddress.cs (1)
EmailAddress(62-68)src/Mailtrap.Abstractions/Emails/Models/Attachment.cs (1)
Attachment(159-174)src/Mailtrap.Abstractions/Emails/Validators/EmailRequestValidator.cs (2)
EmailRequestValidator(12-80)EmailRequestValidator(23-79)
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs (3)
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs (1)
SendEmailRequest(144-152)src/Mailtrap.Abstractions/Emails/Models/EmailAddress.cs (1)
EmailAddress(62-68)src/Mailtrap.Abstractions/Emails/Validators/SendEmailRequestValidator.cs (2)
SendEmailRequestValidator(10-22)SendEmailRequestValidator(14-21)
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs (4)
src/Mailtrap/Configuration/MailtrapJsonSerializerOptions.cs (1)
MailtrapJsonSerializerOptions(4-19)src/Mailtrap.Abstractions/Emails/Requests/SendEmailRequest.cs (1)
SendEmailRequest(71-71)src/Mailtrap/Emails/Requests/SendEmailRequestBuilder.cs (2)
SendEmailRequest(35-44)SendEmailRequest(80-85)src/Mailtrap.Abstractions/Emails/Models/EmailAddress.cs (1)
EmailAddress(62-68)
tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.Attachment.cs (3)
src/Mailtrap/Emails/Requests/EmailRequestBuilder.cs (1)
EmailRequestBuilder(7-679)src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (1)
EmailRequest(194-194)src/Mailtrap.Abstractions/Emails/Models/Attachment.cs (1)
Attachment(159-174)
🪛 markdownlint-cli2 (0.18.1)
docs/cookbook/batch-send-email.md
309-309: Link text should be descriptive
(MD059, descriptive-link-text)
🔇 Additional comments (19)
tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.ReplyTo.cs (1)
1-149: LGTM! Well-structured test refactoring with comprehensive coverage.The test file has been correctly refactored from
SendEmailRequestBuildertoEmailRequestBuilder, with consistent updates to:
- Test fixture and class naming
- Test method naming (using the
Should_XXX_Whenpattern with underscores)- API usage (
EmailRequest.Create()andEmailRequestBuilder.ReplyTo<EmailRequest>())The test coverage is thorough, validating null checks, empty strings, initialization, and override behavior for both
ReplyTo(EmailAddress)andReplyTo(string, string?)overloads.docs/cookbook/batch-send-email.md (2)
176-179: Minor grammar polish for clarity.
[ suggest_optional_refactor ]
Apply this diff:-After creating a batch request, validate it on a client side to ensure it meets API requirements and sending won't throw validation exceptions it also will minimize unnecessary HTTP round-trips. +After creating a batch request, validate it on the client side to ensure it meets API requirements. This helps prevent validation exceptions and minimizes unnecessary HTTP round-trips.
69-72: Docs limit aligns with code BatchEmailRequestValidator enforces a maximum of 500 requests, matching the documentation.docs/cookbook/send-email.md (1)
317-319: LGTM: interface rename to ISendEmailClient.The examples align with the updated public API.
Also applies to: 326-331
src/Mailtrap/Emails/EmailClientEndpointProvider.cs (2)
10-10: Batch segment and URI assembly look correct.Choosing segment via isBatch and appending api/ is clear and consistent with existing Append().
Also applies to: 28-31
22-26: Confirm host selection when inboxId is set.Current logic routes any inbox-scoped request to TestDefaultUrl, ignoring isBulk. If intentional, add/keep a unit test asserting this to prevent regressions.
Also applies to: 32-33
tests/Mailtrap.UnitTests/Emails/EmailClientFactoryTests.cs (2)
62-63: Mocks updated to new endpoint signature — looks good.Setups correctly pass isBatch=false for send paths.
Also applies to: 88-89, 114-115, 140-141, 166-167
76-76: Assert against ISendEmailClient — good.Interface-based assertions keep tests resilient to concrete type changes.
Also applies to: 102-102, 128-128, 154-154, 180-181
tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.From.cs (1)
14-40: From builder tests look solidGood coverage for nulls, initialization, and override semantics. No issues.
src/Mailtrap.Abstractions/Emails/Validators/SendEmailRequestValidator.cs (1)
16-21: Validator composition LGTMApplying
EmailRequestValidatorthenSendEmailRecipientsValidatoris clean and correct.tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.Attachment.cs (1)
171-207: Attachment builder tests are thoroughCovers nulls, optional params, and property initialization. Looks good.
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs (3)
18-27: Serialization round‑trip test looks goodDeserialization-based equivalence is more robust than string compare. LGTM.
100-108: Valid request happy‑path test looks correctCovers From/To/Subject/Text. LGTM.
110-122: Recipient presence validation (positive path) is soundMinimal valid recipients + required fields. LGTM.
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs (5)
141-149: Boundary tests for To length are appropriate1001 fails; 1/500/1000 pass. Clear coverage. LGTM.
Also applies to: 152-160
163-176: Per-item email format checks are preciseIndex-based assertions on collection items are accurate and readable. LGTM.
Also applies to: 179-191
328-338: Attachment validation coverage is solidCovers invalid MimeType and all-valid cases with precise paths. LGTM.
Also applies to: 341-353
362-372: TemplateId mutual exclusivity rules are well specifiedSubject/Text/Html forbidden with TemplateId; happy path passes. LGTM.
Also applies to: 375-385, 388-398, 401-412
478-486: Body rules (null/empty/one/both) comprehensively coveredThorough and unambiguous assertions. LGTM.
Also applies to: 489-500, 503-513, 516-526, 529-540
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (1)
283-286: Past issue resolved.The missing '@' symbol in Cc test email addresses has been correctly fixed. Both tests now use the valid format
recipient{i}@domain.com.Also applies to: 300-302
🧹 Nitpick comments (2)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (2)
816-846: Consider renamingCreateValidBaseRequestfor clarity.The helper method returns an empty
EmailRequestbut the name suggests it would contain valid base properties. Consider renaming toCreateEmptyBaseRequestor populating it with base properties if that better reflects test intent.Alternatively, if the empty base is intentional for testing minimal scenarios, the current implementation is acceptable.
500-500: Consider using consistent assertion style.Line 500 uses
result.IsValid.Should().BeTrue()while other tests (e.g., line 71) useresult.ShouldNotHaveAnyValidationErrors(). Consider standardizing on one approach for consistency.Both styles are correct; this is purely for code consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/cookbook/batch-send-email.md(1 hunks)docs/cookbook/send-email.md(2 hunks)src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs(1 hunks)src/Mailtrap/Emails/Requests/BatchEmailRequestBuilder.cs(1 hunks)tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs(1 hunks)tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.Attachment.cs(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-13T14:27:42.453Z
Learnt from: dr-3lo
PR: railsware/mailtrap-dotnet#158
File: tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs:86-97
Timestamp: 2025-10-13T14:27:42.453Z
Learning: In the Mailtrap .NET client's SendEmailRequest validation, when no recipients are provided (To, Cc, and Bcc are all empty), the validator returns a specific error message: "There should be at least one email recipient added to either To, Cc or Bcc." This message uses lowercase "recipient" rather than a capitalized property name.
Applied to files:
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs
🧬 Code graph analysis (4)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (4)
src/Mailtrap/Emails/Requests/BatchEmailRequestBuilder.cs (6)
BatchEmailRequest(35-44)BatchEmailRequest(70-79)BatchEmailRequest(105-114)BatchEmailRequest(141-151)BatchEmailRequest(177-186)BatchEmailRequest(208-216)src/Mailtrap.Abstractions/Emails/Requests/BatchEmailRequest.cs (1)
BatchEmailRequest(47-47)src/Mailtrap.Abstractions/Emails/Validators/BatchEmailRequestValidator.cs (2)
BatchEmailRequestValidator(9-34)BatchEmailRequestValidator(13-33)src/Mailtrap.Abstractions/Emails/Extensions/BatchEmailRequestExtensions.cs (1)
SendEmailRequest(31-54)
src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (3)
src/Mailtrap.Abstractions/Emails/Models/EmailAddress.cs (1)
EmailAddress(62-68)src/Mailtrap.Abstractions/Emails/Models/Attachment.cs (1)
Attachment(159-174)src/Mailtrap.Abstractions/Emails/Validators/EmailRequestValidator.cs (2)
EmailRequestValidator(12-80)EmailRequestValidator(23-79)
src/Mailtrap/Emails/Requests/BatchEmailRequestBuilder.cs (4)
src/Mailtrap.Abstractions/Emails/Requests/BatchEmailRequest.cs (1)
BatchEmailRequest(47-47)examples/Mailtrap.Example.Email.BatchSend/Program.cs (2)
BatchEmailRequest(106-136)BatchEmailRequest(142-166)src/Mailtrap.Abstractions/Core/Extensions/Ensure.cs (2)
Ensure(9-106)NotNull(18-33)src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (1)
EmailRequest(194-194)
tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.Attachment.cs (3)
src/Mailtrap/Emails/Requests/EmailRequestBuilder.cs (1)
EmailRequestBuilder(7-679)src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (1)
EmailRequest(194-194)src/Mailtrap.Abstractions/Emails/Models/Attachment.cs (1)
Attachment(159-174)
🔇 Additional comments (21)
docs/cookbook/send-email.md (2)
300-303: Correct exception type spellingRenaming to
OperationCanceledExceptionmatches the actual .NET type; looks good.
317-331: Updated interface references read wellSwitching to
ISendEmailClientkeeps the samples aligned with the new client API.tests/Mailtrap.UnitTests/Emails/Requests/EmailRequestBuilderTests.Attachment.cs (1)
1-208: LGTM! Well-executed refactoring with comprehensive test coverage.The transition from
SendEmailRequesttoEmailRequestand the corresponding builder update is done correctly throughout this test file. The tests comprehensively cover:
- Null validation for request and parameters (lines 17-22, 25-32, 98-143)
- Empty parameter handling (lines 35-42, 66-75)
- Single and multiple attachment addition (lines 45-48, 51-63)
- Both
Attachoverloads with all parameter combinations (lines 146-205)- Correct use of explicit generic type parameters when calling static methods with null arguments (lines 19, 100)
The consistent test naming convention (
Attach_Should_...) improves readability, and the helper methodAttach_CreateAndValidatecorrectly returnsEmailRequestinstead of the old type.tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (10)
1-10: LGTM!Test setup with clear, reusable constants for valid/invalid test data.
11-85: LGTM!Comprehensive coverage of Requests collection validation including null, empty, and count limit scenarios. Proper use of parameterized tests for boundary conditions.
88-145: LGTM!Thorough validation of individual request items including null checks and recipient requirements. Tests correctly verify that at least one of To, Cc, or Bcc must be present.
149-214: LGTM!Proper validation of From and ReplyTo fields with correct email format checking and property path assertions.
218-404: LGTM!Comprehensive validation of recipient collections (To, Cc, Bcc) with proper count limits (1000), format validation, and boundary testing. All email formats are correct.
408-440: LGTM!Proper validation of attachment collections with correct property path assertions for indexed items.
444-523: LGTM!Critical validation logic ensuring templates and content fields (Subject, Text, Html) are mutually exclusive. Tests thoroughly cover both per-item and base template scenarios.
527-601: LGTM!Thorough Subject validation covering required field checks, base request merging, and template conflicts. Good use of parameterized tests for null/empty scenarios.
605-665: LGTM!Proper Category validation with length limits (255) and base request merging. Good use of random string generation for boundary testing.
669-814: LGTM!Exceptional coverage of Body validation with all combinations of Text/Html presence, base request merging, and template conflicts. Tests ensure at least one body format is always required.
src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (1)
1-203: LGTM overall with documentation clarification needed.The EmailRequest record is well-structured with:
- Proper JSON serialization attributes and property ordering
- Comprehensive XML documentation for all properties
- Nullable reference types used appropriately
- Collection properties initialized with default empty instances
- Factory method and validation implementation
The only concern is the Category property documentation inconsistency noted above.
src/Mailtrap/Emails/Requests/BatchEmailRequestBuilder.cs (7)
35-44: LGTM! Proper null checks and fluent chaining.The params overload correctly validates all arguments and uses AddRange for efficient bulk addition.
70-79: LGTM! IEnumerable overload complements params version.Good use of
inheritdocto avoid documentation duplication while maintaining the same validation and chaining pattern.
105-114: LGTM! Single request overload for convenience.Clean implementation for adding individual requests with consistent null checking.
141-151: LGTM! Inline configuration is a nice fluent API touch.The Action overload enables clean inline request configuration, creating a more expressive builder syntax as shown in the example code.
177-186: LGTM! Base configuration follows the same fluent pattern.Consistent with the Requests overloads, allowing inline Base request configuration.
208-216: LGTM! Direct Base assignment completes the API.Provides flexibility to set a pre-configured EmailRequest as the Base.
1-219: Excellent fluent builder implementation!This builder class demonstrates solid design principles:
- Comprehensive method overloads for different usage patterns
- Consistent null checking using Ensure.NotNull
- Proper fluent chaining with return values
- Well-organized with regions
- Thorough XML documentation with effective use of inheritdoc
- Aligns perfectly with the example usage shown in Program.cs
The multiple overloads (params, IEnumerable, single item, Action configuration) provide excellent flexibility for consumers.
src/Mailtrap.Abstractions/Emails/Validators/SendEmailRecipientsValidator.cs
Outdated
Show resolved
Hide resolved
- Updated limits for Email Send Request recipients: now sum of To + Cc + Bcc should not exceed 1000 either. Appropriate validators, tests and docs are corrected. - small improvements for batch email send cookbook - correction for Batch Email Send example project
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs (1)
26-35: Tighten assertions to target “Recipients” property
ShouldNotHaveValidationErrorFor(r => r)can mask recipient-rule failures. Assert against the “Recipients” key to verify the intended validation.Apply:
- result.ShouldNotHaveValidationErrorFor(r => r); + result.ShouldNotHaveValidationErrorFor("Recipients");Do this in:
- Validation_Should_Pass_WhenOnlyToRecipientsPresent
- Validation_Should_Pass_WhenOnlyCcRecipientsPresent
- Validation_Should_Pass_WhenOnlyBccRecipientsPresent
Also applies to: 38-47, 50-59
🧹 Nitpick comments (2)
docs/cookbook/send-email.md (1)
327-328: Tighten the grammar in the tipLine 327 reads “it will be good idea”; please add the missing article (and comma) so it becomes “it will be a good idea,” which matches standard English usage. Suggest applying:
- > Thus in case when you need to perform multiple `Send()` calls to the same endpoint it will be good idea + > Thus, if you need to perform multiple `Send()` calls to the same endpoint, it will be a good ideaBased on static analysis hints.
src/Mailtrap.Abstractions/Emails/Requests/SendEmailRequest.cs (1)
20-22: Doc clarity: use “across To, Cc, and Bcc” (and fix “Gets or sets”)Minor doc fixes for accuracy/consistency.
Apply:
- /// Gets a collection of <see cref="EmailAddress"/> objects, defining who will receive a copy of email. + /// Gets or sets a collection of <see cref="EmailAddress"/> objects, defining who will receive a copy of email. @@ - /// At least one recipient must be specified in one of the collections: <see cref="To"/>, <see cref="Cc"/> or <see cref="Bcc"/>.<br /> - /// The sum of recipients in <see cref="To"/>, <see cref="Cc"/> or <see cref="Bcc"/> must not exceed 1000 recipients. + /// At least one recipient must be specified in one of the collections: <see cref="To"/>, <see cref="Cc"/> or <see cref="Bcc"/>.<br /> + /// The total recipients across <see cref="To"/>, <see cref="Cc"/> and <see cref="Bcc"/> must not exceed 1000.Repeat the same “Gets or sets …” and “across … and …” wording updates in the Cc and Bcc property docs.
Also applies to: 38-40, 56-58
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
docs/cookbook/batch-send-email.md(1 hunks)docs/cookbook/send-email.md(3 hunks)examples/Mailtrap.Example.Email.BatchSend/Program.cs(1 hunks)examples/Mailtrap.Example.Email.Send/Program.cs(2 hunks)src/Mailtrap.Abstractions/Emails/Requests/SendEmailRequest.cs(3 hunks)src/Mailtrap.Abstractions/Emails/Validators/SendEmailRecipientsValidator.cs(1 hunks)src/Mailtrap.Abstractions/Emails/Validators/SendEmailRequestValidator.cs(1 hunks)tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs(1 hunks)tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs(33 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- examples/Mailtrap.Example.Email.Send/Program.cs
- examples/Mailtrap.Example.Email.BatchSend/Program.cs
- tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs
- src/Mailtrap.Abstractions/Emails/Validators/SendEmailRequestValidator.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-13T14:27:42.453Z
Learnt from: dr-3lo
PR: railsware/mailtrap-dotnet#158
File: tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs:86-97
Timestamp: 2025-10-13T14:27:42.453Z
Learning: In the Mailtrap .NET client's SendEmailRequest validation, when no recipients are provided (To, Cc, and Bcc are all empty), the validator returns a specific error message: "There should be at least one email recipient added to either To, Cc or Bcc." This message uses lowercase "recipient" rather than a capitalized property name.
Applied to files:
docs/cookbook/send-email.mdsrc/Mailtrap.Abstractions/Emails/Requests/SendEmailRequest.cssrc/Mailtrap.Abstractions/Emails/Validators/SendEmailRecipientsValidator.cstests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs
📚 Learning: 2025-09-25T13:44:20.706Z
Learnt from: dr-3lo
PR: railsware/mailtrap-dotnet#153
File: src/Mailtrap.Abstractions/ContactImports/Validators/CreateContactImportRequestValidator.cs:22-30
Timestamp: 2025-09-25T13:44:20.706Z
Learning: In the Mailtrap .NET repository, validators follow a standard pattern: they rely on FluentValidation's default error messages for simple validation rules (NotEmpty, Length, MaximumLength, etc.) rather than using custom WithMessage calls. Custom messages are only used for complex business logic validations. All validators have comprehensive XML documentation, a static Instance property for reuse, and follow consistent constructor patterns.
Applied to files:
src/Mailtrap.Abstractions/Emails/Validators/SendEmailRecipientsValidator.cs
🧬 Code graph analysis (3)
src/Mailtrap.Abstractions/Emails/Requests/SendEmailRequest.cs (5)
examples/Mailtrap.Example.Email.BatchSend/Program.cs (6)
SendEmailRequest(169-189)SendEmailRequest(195-204)SendEmailRequest(210-222)SendEmailRequest(227-265)SendEmailRequest(270-326)SendEmailRequest(331-405)examples/Mailtrap.Example.Email.Send/Program.cs (6)
SendEmailRequest(66-86)SendEmailRequest(92-101)SendEmailRequest(107-119)SendEmailRequest(124-159)SendEmailRequest(164-220)SendEmailRequest(225-299)tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (2)
SendEmailRequest(877-890)EmailRequest(872-875)src/Mailtrap/Emails/Requests/SendEmailRequestBuilder.cs (3)
SendEmailRequest(35-44)SendEmailRequest(80-85)SendEmailRequest(117-126)src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (2)
EmailRequest(194-194)ValidationResult(198-203)
src/Mailtrap.Abstractions/Emails/Validators/SendEmailRecipientsValidator.cs (3)
examples/Mailtrap.Example.Email.BatchSend/Program.cs (6)
SendEmailRequest(169-189)SendEmailRequest(195-204)SendEmailRequest(210-222)SendEmailRequest(227-265)SendEmailRequest(270-326)SendEmailRequest(331-405)src/Mailtrap.Abstractions/Emails/Requests/SendEmailRequest.cs (1)
SendEmailRequest(71-71)tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (1)
SendEmailRequest(877-890)
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs (3)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (24)
TestFixture(4-893)SendEmailRequest(877-890)Test(12-24)Test(26-36)Test(38-48)Test(50-60)Test(62-72)Test(74-83)Test(90-99)Test(101-110)Test(112-121)Test(123-132)Test(134-143)Test(151-159)Test(161-171)Test(179-189)Test(191-200)Test(202-212)TestCase(421-436)TestCase(438-450)TestCase(576-587)TestCase(589-605)TestCase(619-633)TestCase(680-695)tests/Mailtrap.UnitTests/Emails/Models/EmailAddressValidatorTests.cs (2)
TestFixture(4-53)TestCase(37-52)src/Mailtrap.Abstractions/Emails/Validators/SendEmailRequestValidator.cs (2)
SendEmailRequestValidator(10-22)SendEmailRequestValidator(14-21)
🪛 LanguageTool
docs/cookbook/send-email.md
[grammar] ~327-~327: There might be a mistake here.
Context: ...d()` calls to the same endpoint it will be good idea > to spawn client once and th...
(QB_NEW_EN)
[grammar] ~328-~328: There might be a mistake here.
Context: ...same endpoint it will be good idea > to spawn client once and then reuse it: > ```csh...
(QB_NEW_EN)
docs/cookbook/batch-send-email.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...h-send-email --- # Sending Batch Emails This article covers scenarios for formin...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...")); ... ``` ### Regular initialization Alternatively, you can use object initia...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...t unless overridden. ## Attaching files You can attach files to Base and individ...
(QB_NEW_EN)
[grammar] ~129-~129: There might be a mistake here.
Context: ...ohn" } })); ``` ## Kitchen sink Overview of all possible settings for ba...
(QB_NEW_EN)
[grammar] ~183-~183: There might be a mistake here.
Context: ...tom Value")); ``` ## Request validation After creating a batch request, validate...
(QB_NEW_EN)
[grammar] ~184-~184: There might be a mistake here.
Context: ...l minimize unnecessary HTTP round-trips. @Mailtrap.Emails.Requests.BatchEmailRequ...
(QB_NEW_EN)
[grammar] ~225-~225: There might be a mistake here.
Context: ...andle validation issues } ``` > [!NOTE] > The client validates batch requests be...
(QB_NEW_EN)
[grammar] ~291-~291: There might be a mistake here.
Context: ...guration. Additionally, you can always use specific send API (transactional, bulk ...
(QB_NEW_EN)
[grammar] ~304-~304: There might be a mistake here.
Context: ...d()` calls to the same endpoint it will be good idea > to spawn client once and th...
(QB_NEW_EN)
[grammar] ~305-~305: There might be a mistake here.
Context: ...same endpoint it will be good idea > to spawn client once and then reuse it: > ```csh...
(QB_NEW_EN)
🔇 Additional comments (1)
src/Mailtrap.Abstractions/Emails/Validators/SendEmailRecipientsValidator.cs (1)
34-43: Aggregated “Recipients” rules: LGTMThe “at least one recipient” and “total recipients ≤ 1000” checks with a single “Recipients” error key and clear messages look correct and align with our validation patterns. [Based on learnings]
src/Mailtrap.Abstractions/Emails/Validators/SendEmailRecipientsValidator.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs (2)
200-232: Consider using consistent approach with To tests.The Cc validation tests are correct and the email addresses now include the
@symbol. However, note that these tests use loops with the fluent API (request.Cc($"recipient{i}@domain.com")), while the To tests useEnumerable.Repeatwith direct assignment. While both approaches test valid scenarios, using a consistent approach across similar tests could improve maintainability.If you prefer consistency, apply this diff to align with the To tests:
[Test] public void Validation_Should_Fail_WhenCcLengthExceedsLimit([Values(1001)] int count) { var request = SendEmailRequest.Create(); - - for (var i = 1; i <= count; i++) - { - request.Cc($"recipient{i}@domain.com"); - } + request.Cc = Enumerable.Repeat(new EmailAddress(_validEmail), count).ToList(); var result = SendEmailRequestValidator.Instance.TestValidate(request); result.ShouldHaveValidationErrorFor("Recipients"); result.ShouldHaveValidationErrorFor(r => r.Cc); } [Test] public void Validation_Should_Pass_WhenCcLengthWithinLimit([Values(1, 500, 1000)] int count) { var request = SendEmailRequest.Create(); - - for (var i = 1; i <= count; i++) - { - request.Cc($"recipient{i}@domain.com"); - } + request.Cc = Enumerable.Repeat(new EmailAddress(_validEmail), count).ToList(); var result = SendEmailRequestValidator.Instance.TestValidate(request); result.ShouldNotHaveValidationErrorFor("Recipients"); result.ShouldNotHaveValidationErrorFor(r => r.Cc); }
267-299: Consider using consistent approach with To tests.The Bcc validation tests are correct and the email addresses now include the
@symbol. Similar to the Cc tests, these use loops with the fluent API while To tests useEnumerable.Repeat. Consider applying the same refactoring suggested for Cc tests to maintain consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs(1 hunks)tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.cs(1 hunks)tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs(33 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-13T14:27:42.453Z
Learnt from: dr-3lo
PR: railsware/mailtrap-dotnet#158
File: tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs:86-97
Timestamp: 2025-10-13T14:27:42.453Z
Learning: In the Mailtrap .NET client's SendEmailRequest validation, when no recipients are provided (To, Cc, and Bcc are all empty), the validator returns a specific error message: "There should be at least one email recipient added to either To, Cc or Bcc." This message uses lowercase "recipient" rather than a capitalized property name.
Applied to files:
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cstests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs
📚 Learning: 2025-09-04T12:23:59.276Z
Learnt from: dr-3lo
PR: railsware/mailtrap-dotnet#139
File: tests/Mailtrap.IntegrationTests/Contacts/Update_Success.json:16-17
Timestamp: 2025-09-04T12:23:59.276Z
Learning: Test fixtures in the Mailtrap .NET client should accurately represent the actual server response format and should not be modified to match client-side converters or serialization preferences.
Applied to files:
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs
🧬 Code graph analysis (2)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (5)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.cs (1)
BatchEmailRequest(71-92)tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Base.cs (1)
SendEmailRequest(259-272)src/Mailtrap/Emails/Requests/BatchEmailRequestBuilder.cs (6)
BatchEmailRequest(35-44)BatchEmailRequest(70-79)BatchEmailRequest(105-114)BatchEmailRequest(141-151)BatchEmailRequest(177-186)BatchEmailRequest(208-216)src/Mailtrap.Abstractions/Emails/Validators/BatchEmailRequestValidator.cs (2)
BatchEmailRequestValidator(9-34)BatchEmailRequestValidator(13-33)src/Mailtrap.Abstractions/Emails/Extensions/BatchEmailRequestExtensions.cs (1)
SendEmailRequest(31-54)
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs (4)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (23)
SendEmailRequest(891-904)Test(12-24)Test(26-36)Test(38-48)Test(50-60)Test(62-72)Test(74-83)Test(90-99)Test(101-110)Test(112-121)Test(123-132)Test(134-143)Test(151-159)Test(161-171)Test(179-189)Test(191-200)Test(202-212)TestCase(434-450)TestCase(452-464)TestCase(590-601)TestCase(603-619)TestCase(633-647)TestCase(694-709)src/Mailtrap.Abstractions/Emails/Requests/SendEmailRequest.cs (1)
SendEmailRequest(71-71)src/Mailtrap.Abstractions/Emails/Models/EmailAddress.cs (1)
EmailAddress(62-68)src/Mailtrap.Abstractions/Emails/Validators/SendEmailRequestValidator.cs (2)
SendEmailRequestValidator(10-22)SendEmailRequestValidator(14-21)
🔇 Additional comments (20)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (13)
1-10: LGTM!The test fixture setup and private fields are appropriate for consistent test data across the suite.
11-85: LGTM!The Requests collection validation tests comprehensively cover null, empty, count limits, and scenarios with/without a base request.
88-145: LGTM!The request items validation tests correctly validate null items and recipient presence requirements across To, Cc, and Bcc fields.
149-173: LGTM!The From field validation tests correctly validate email format for both invalid and valid cases.
177-214: LGTM!The ReplyTo field validation tests appropriately handle null values and validate email format.
218-279: LGTM!The To recipients validation tests thoroughly cover count limits and individual email format validation. The comment explaining hardcoded property names for collection indexers is helpful.
283-347: LGTM!The Cc recipients validation tests correctly use valid email format (
recipient{i}@domain.com) and comprehensively test count limits and individual email validation.
351-416: LGTM!The Bcc recipients validation tests mirror the To and Cc test patterns and correctly validate count limits and email format.
469-501: LGTM!The attachments validation tests correctly validate MimeType requirements for individual attachments within batch requests.
505-584: LGTM!The Template ID validation tests comprehensively verify that Subject, TextBody, and HtmlBody are forbidden when a template is used, including cases where these properties are inherited from the base request.
588-662: LGTM!The Subject validation tests thoroughly cover standalone validation, base request merging, and template interaction constraints.
666-726: LGTM!The Category validation tests correctly enforce the 255-character limit and properly test base request merging behavior.
730-906: LGTM!The Body validation tests comprehensively verify that either TextBody or HtmlBody must be provided, correctly handle base request merging, and enforce template constraints. The helper methods provide well-structured test data.
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs (7)
1-10: LGTM!The namespace, class name, and field declarations are correct. The typo fix in
_validEmail(domain.com instead of domean.com) is a good catch.
13-134: LGTM!The Request, From, and ReplyTo validation tests are well-structured and comprehensive. They correctly test the various scenarios including:
- No recipients present
- Single recipient type (To, Cc, or Bcc only)
- Invalid and valid sender/reply-to emails
138-196: LGTM!The To recipient validation tests are comprehensive and correct. The use of
Enumerable.Repeatwith parameterized values effectively tests the boundary conditions (1, 500, 1000, 1001).
234-262: LGTM!The Cc email validation tests correctly verify invalid and valid email address scenarios.
301-328: LGTM!The Bcc email validation tests correctly verify invalid and valid email address scenarios.
364-377: LGTM!The test correctly verifies that exactly 1000 total recipients is within the allowed limit. This is a good boundary condition test.
379-596: LGTM!The remaining validation tests are comprehensive and well-structured:
- Attachments: Correctly tests invalid and valid attachment scenarios
- Templated: Properly validates that forbidden properties (Subject, TextBody, HtmlBody) cannot be set when using a template
- Subject: Tests null and provided subject validation
- Category: Tests maximum length constraint (255 characters)
- Body: Comprehensively tests that at least one body type (Text or HTML) must be provided
All assertions are appropriate and the test coverage is thorough.
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs
Show resolved
Hide resolved
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.cs (3)
71-92: Clarify and explicitly test Base request merging behavior.The
CreateValidRequesthelper setsFromandSubjectin both theBase(lines 75-76) and the individualSendEmailRequest(lines 82, 84), while the individual request omitsHtmldespite it being required (per line 57). This suggests that the Base provides defaults or merges with individual requests, but this merging behavior is only implicitly tested rather than explicitly verified.Consider:
- Adding an explicit test that verifies field inheritance from
Baseto individual requests (e.g., when an individual request omitsHtml, it's inherited fromBase).- Adding a test that verifies override behavior (e.g., when an individual request sets
Subject, it overrides theBasevalue).- Simplifying this helper by removing redundant field assignments if inheritance is the intended behavior, or adding a comment explaining the override scenario being tested.
Based on the AI summary, the
BatchEmailRequestValidatorvalidates "base vs. per-request merges," but without explicit tests, it's unclear whether this helper is intentionally testing override behavior or simply over-specifying fields.
18-18: Consider using consistent test naming convention.This test is named
Should_SerializeAndDeserializeCorrectly, while others follow theAction_Should_Behavior_When_Conditionpattern (e.g., line 8:Create_ShouldReturnNewInstance_WhenCalled). For consistency, consider renaming to something likeSerializeAndDeserialize_Should_PreserveData_WhenRoundTrippedor adjusting other tests to match this style.
1-93: Consider adding tests for additional batch scenarios.The current test suite covers basic creation, serialization, and validation of single-request batches. To ensure robustness, consider adding tests for:
- Multiple valid requests in a single batch (e.g., 2-3 requests).
- Mixed valid and invalid requests in a batch (verifying that validation aggregates errors from all contained requests).
- Edge cases such as maximum batch size limits (if applicable) or duplicate recipient handling.
These scenarios would provide more comprehensive coverage of the batch send functionality.
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs (2)
364-375: Consider adding test cases well below the limit.The current test only verifies the boundary case (exactly 1000 recipients). Adding cases like
[TestCase(1, 0, 0)],[TestCase(100, 100, 100)], or[TestCase(0, 0, 1)]would strengthen coverage by ensuring the validator correctly accepts valid recipient counts throughout the range, not just at the upper boundary.Example addition:
+ [TestCase(1, 0, 0)] + [TestCase(100, 100, 100)] [TestCase(500, 400, 100)] public void Validation_Requests_Should_Pass_WhenTotalRecipientsWithinLimit(int toCount, int ccCount, int bccCount)
144-144: Prefer builder pattern for consistency with other tests.This test uses direct property assignment (
request.To = ...) while other similar tests (e.g., lines 355, 368) use the builder pattern (.To(...)). For consistency and to better exercise the public fluent API, consider using the builder pattern throughout.Apply this diff for consistency:
var request = SendEmailRequest.Create(); - request.To = Enumerable.Repeat(new EmailAddress(_validEmail), count).ToList(); + request.To(Enumerable.Repeat(new EmailAddress(_validEmail), count).ToArray());Also applies to: 156-156
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs(1 hunks)tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.cs(1 hunks)tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs(33 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-13T14:27:42.453Z
Learnt from: dr-3lo
PR: railsware/mailtrap-dotnet#158
File: tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs:86-97
Timestamp: 2025-10-13T14:27:42.453Z
Learning: In the Mailtrap .NET client's SendEmailRequest validation, when no recipients are provided (To, Cc, and Bcc are all empty), the validator returns a specific error message: "There should be at least one email recipient added to either To, Cc or Bcc." This message uses lowercase "recipient" rather than a capitalized property name.
Applied to files:
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs
🧬 Code graph analysis (2)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.cs (4)
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs (1)
SendEmailRequest(144-152)src/Mailtrap.Abstractions/Emails/Requests/BatchEmailRequest.cs (1)
BatchEmailRequest(47-47)src/Mailtrap/Configuration/MailtrapJsonSerializerOptions.cs (1)
MailtrapJsonSerializerOptions(4-19)src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (1)
EmailRequest(194-194)
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs (4)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (18)
TestFixture(4-907)SendEmailRequest(891-904)Test(12-24)Test(26-36)Test(38-48)Test(50-60)Test(62-72)Test(74-83)Test(90-99)Test(101-110)Test(112-121)Test(123-132)Test(134-143)Test(151-159)Test(161-171)Test(179-189)Test(191-200)Test(202-212)tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs (1)
TestFixture(4-153)tests/Mailtrap.UnitTests/Emails/Models/EmailAddressValidatorTests.cs (1)
TestFixture(4-53)src/Mailtrap.Abstractions/Emails/Validators/SendEmailRequestValidator.cs (2)
SendEmailRequestValidator(10-22)SendEmailRequestValidator(14-21)
🔇 Additional comments (1)
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs (1)
7-7: Good catch on the typo fix.The correction from "domean.com" to "domain.com" ensures valid test data is used throughout the test suite.
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
docs/cookbook/batch-send-email.md (6)
184-186: Tighten the sentence; avoid run-on.-After creating a batch request, validate it on a client side to ensure it meets API requirements and sending won't throw validation exceptions it also will minimize unnecessary HTTP round-trips. +After creating a batch request, validate it on the client side to ensure it meets API requirements and to avoid validation exceptions at send time. This also minimizes unnecessary HTTP round-trips.
80-81: Clarify attachment phrasing.-You can attach files to Base and individual requests in the batch, just as with single emails in two ways for inlining (embedding) or as a standalone downloadable file: +You can attach files to the Base and individual requests in the batch, just as with single emails—either inline (embedded) or as standalone downloadable files:
291-299: Minor wording: capitalize API names and simplify phrasing.-Additionally, you can always use specific send API (transactional, bulk or test) explicitly, to route emails to: +Additionally, you can explicitly use a specific Send API (Transactional, Bulk, or Test) to route emails:
304-306: Polish wording in the TIP.-> Thus, if you need to perform multiple `Send()` calls to the same endpoint, it will be a good idea +> Thus, if you need to perform multiple `Send()` calls to the same endpoint, it is a good idea > to spawn client once and then reuse it:Optionally:
-> to spawn client once and then reuse it: +> to create the client once and then reuse it:
28-31: Nit: clean display name.- .From("[email protected]", " It's a John Cena") // Overwrite the base attributes + .From("[email protected]", "John Cena") // Overwrite the base attributes
48-50: Minor: spacing and consistency in note.-// You can specify up to 1000 recipients in total across the To, Cc, and Bcc fields (i.e. To + CC + Bcc <=1000). +// You can specify up to 1000 recipients in total across the To, Cc, and Bcc fields (i.e., To + Cc + Bcc <= 1000).docs/cookbook/send-email.md (2)
325-327: Polish wording in TIP and grammar.-> are factory methods that will create new @Mailtrap.Emails.ISendEmailClient instance every time when called. +> are factory methods that create a new @Mailtrap.Emails.ISendEmailClient instance each time they are called.-> Thus, if you need to perform multiple `Send()` calls to the same endpoint, it will be a good idea +> Thus, if you need to perform multiple `Send()` calls to the same endpoint, it is a good ideaOptionally:
- to spawn client once and then reuse it: + to create the client once and reuse it:
48-49: Minor: spacing and consistency in note.-// You can specify up to 1000 recipients in total across the To, Cc, and Bcc fields (i.e. To + CC + Bcc <=1000). +// You can specify up to 1000 recipients in total across the To, Cc, and Bcc fields (i.e., To + Cc + Bcc <= 1000).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/cookbook/batch-send-email.md(1 hunks)docs/cookbook/send-email.md(3 hunks)src/Mailtrap.Abstractions/Emails/Requests/SendEmailRequest.cs(1 hunks)src/Mailtrap.Abstractions/Emails/Validators/SendEmailRecipientsValidator.cs(1 hunks)tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs(1 hunks)tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs(34 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-13T14:27:42.453Z
Learnt from: dr-3lo
PR: railsware/mailtrap-dotnet#158
File: tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs:86-97
Timestamp: 2025-10-13T14:27:42.453Z
Learning: In the Mailtrap .NET client's SendEmailRequest validation, when no recipients are provided (To, Cc, and Bcc are all empty), the validator returns a specific error message: "There should be at least one email recipient added to either To, Cc or Bcc." This message uses lowercase "recipient" rather than a capitalized property name.
Applied to files:
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cssrc/Mailtrap.Abstractions/Emails/Validators/SendEmailRecipientsValidator.cssrc/Mailtrap.Abstractions/Emails/Requests/SendEmailRequest.cstests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs
📚 Learning: 2025-09-25T13:44:20.706Z
Learnt from: dr-3lo
PR: railsware/mailtrap-dotnet#153
File: src/Mailtrap.Abstractions/ContactImports/Validators/CreateContactImportRequestValidator.cs:22-30
Timestamp: 2025-09-25T13:44:20.706Z
Learning: In the Mailtrap .NET repository, validators follow a standard pattern: they rely on FluentValidation's default error messages for simple validation rules (NotEmpty, Length, MaximumLength, etc.) rather than using custom WithMessage calls. Custom messages are only used for complex business logic validations. All validators have comprehensive XML documentation, a static Instance property for reuse, and follow consistent constructor patterns.
Applied to files:
src/Mailtrap.Abstractions/Emails/Validators/SendEmailRecipientsValidator.cs
🧬 Code graph analysis (4)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (5)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Base.cs (3)
TestFixture(5-275)List(253-257)SendEmailRequest(259-272)src/Mailtrap.Abstractions/Emails/Requests/BatchEmailRequest.cs (1)
BatchEmailRequest(47-47)src/Mailtrap/Emails/Requests/BatchEmailRequestBuilder.cs (6)
BatchEmailRequest(35-44)BatchEmailRequest(70-79)BatchEmailRequest(105-114)BatchEmailRequest(141-151)BatchEmailRequest(177-186)BatchEmailRequest(208-216)src/Mailtrap.Abstractions/Emails/Validators/BatchEmailRequestValidator.cs (2)
BatchEmailRequestValidator(9-34)BatchEmailRequestValidator(13-33)src/Mailtrap.Abstractions/Emails/Extensions/BatchEmailRequestExtensions.cs (1)
SendEmailRequest(31-54)
src/Mailtrap.Abstractions/Emails/Validators/SendEmailRecipientsValidator.cs (4)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (1)
SendEmailRequest(908-921)src/Mailtrap.Abstractions/Emails/Requests/SendEmailRequest.cs (1)
SendEmailRequest(71-71)examples/Mailtrap.Example.Email.Send/Program.cs (6)
SendEmailRequest(66-86)SendEmailRequest(92-101)SendEmailRequest(107-119)SendEmailRequest(124-159)SendEmailRequest(164-220)SendEmailRequest(225-299)src/Mailtrap.Abstractions/Emails/Extensions/BatchEmailRequestExtensions.cs (1)
SendEmailRequest(31-54)
src/Mailtrap.Abstractions/Emails/Requests/SendEmailRequest.cs (4)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (2)
SendEmailRequest(908-921)EmailRequest(903-906)src/Mailtrap/Emails/Requests/SendEmailRequestBuilder.cs (3)
SendEmailRequest(35-44)SendEmailRequest(80-85)SendEmailRequest(117-126)src/Mailtrap.Abstractions/Emails/Requests/EmailRequest.cs (2)
EmailRequest(194-194)ValidationResult(198-203)src/Mailtrap.Abstractions/Emails/Models/EmailAddress.cs (1)
EmailAddress(62-68)
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs (5)
tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (23)
Test(12-24)Test(26-36)Test(38-48)Test(50-60)Test(62-72)Test(74-83)Test(90-99)Test(101-110)Test(112-121)Test(123-132)Test(134-143)Test(151-159)Test(161-171)Test(179-189)Test(191-200)Test(202-212)SendEmailRequest(908-921)TestCase(452-467)TestCase(469-481)TestCase(607-618)TestCase(620-636)TestCase(650-664)TestCase(711-726)tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.cs (1)
TestCase(124-142)src/Mailtrap.Abstractions/Emails/Requests/SendEmailRequest.cs (1)
SendEmailRequest(71-71)src/Mailtrap.Abstractions/Emails/Models/EmailAddress.cs (1)
EmailAddress(62-68)src/Mailtrap.Abstractions/Emails/Validators/SendEmailRequestValidator.cs (2)
SendEmailRequestValidator(10-22)SendEmailRequestValidator(14-21)
🪛 LanguageTool
docs/cookbook/batch-send-email.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...h-send-email --- # Sending Batch Emails This article covers scenarios for formin...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...")); ... ``` ### Regular initialization Alternatively, you can use object initia...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...t unless overridden. ## Attaching files You can attach files to Base and individ...
(QB_NEW_EN)
[grammar] ~129-~129: There might be a mistake here.
Context: ...ohn" } })); ``` ## Kitchen sink Overview of all possible settings for ba...
(QB_NEW_EN)
[grammar] ~183-~183: There might be a mistake here.
Context: ...tom Value")); ``` ## Request validation After creating a batch request, validate...
(QB_NEW_EN)
[grammar] ~184-~184: There might be a mistake here.
Context: ...l minimize unnecessary HTTP round-trips. @Mailtrap.Emails.Requests.BatchEmailRequ...
(QB_NEW_EN)
[grammar] ~225-~225: There might be a mistake here.
Context: ...andle validation issues } ``` > [!NOTE] > The client validates batch requests be...
(QB_NEW_EN)
[grammar] ~291-~291: There might be a mistake here.
Context: ...guration. Additionally, you can always use specific send API (transactional, bulk ...
(QB_NEW_EN)
[grammar] ~305-~305: There might be a mistake here.
Context: ...e endpoint, it will be a good idea > to spawn client once and then reuse it: > ```csh...
(QB_NEW_EN)
docs/cookbook/send-email.md
[grammar] ~328-~328: There might be a mistake here.
Context: ...e endpoint, it will be a good idea > to spawn client once and then reuse it: > ```csh...
(QB_NEW_EN)
🔇 Additional comments (8)
src/Mailtrap.Abstractions/Emails/Requests/SendEmailRequest.cs (1)
9-61: LGTM!The recipient properties (To, Cc, Bcc) are well-structured with consistent documentation, appropriate JsonPropertyOrder values, and modern collection initialization using
= []. The documentation accurately reflects the validation rules including the requirement for at least one recipient and the total limit of 1000 recipients across all lists.tests/Mailtrap.UnitTests/Emails/Requests/BatchEmailRequestTests.Validator.Requests.cs (1)
1-924: Excellent test coverage!The test suite comprehensively validates all aspects of the BatchEmailRequest including:
- Request list constraints (null, empty, count limits)
- Individual request validation (From, ReplyTo, recipients)
- Recipient count limits per list and in aggregate
- Attachment validation
- Template ID conflict detection
- Subject, category, and body validation
- Base/request merging scenarios
The tests are well-organized with clear naming and appropriate use of test data.
src/Mailtrap.Abstractions/Emails/Validators/SendEmailRecipientsValidator.cs (1)
4-44: Well-structured recipient validator!The validator correctly implements recipient validation with:
- Null-safe per-list count checks preventing NullReferenceException
- Individual email address validation for each recipient
- Enforcement of "at least one recipient" rule with appropriate error message
- Total recipient limit (1000) across To, Cc, and Bcc lists
The custom error messages align with the established patterns in the codebase.
Based on learnings
tests/Mailtrap.UnitTests/Emails/Requests/SendEmailRequestTests.Validator.cs (2)
7-7: Good catch on the typo!Fixed the email address from
[email protected]to[email protected].
335-396: Excellent addition of total recipient validation tests!The new test section comprehensively validates the aggregate recipient limit:
- Tests for zero recipients (both empty and null collections)
- Tests various combinations that exceed 1000 total recipients
- Tests edge cases at exactly 1000 recipients
The test cases properly exercise the validation rules without including semantically incorrect scenarios like
[TestCase(0, 0, 0)]in the "exceeds limit" test.docs/cookbook/batch-send-email.md (1)
278-281: Correct exception type used.Using OperationCanceledException is correct for cancellation.
docs/cookbook/send-email.md (2)
300-303: Correct exception type used.OperationCanceledException is the proper .NET type.
317-319: API rename looks correct.ISendEmailClient usage aligns with IMailtrapClient factory methods.
3 participants
Motivation
Batch Send API support into .NET client #95
Changes
How to test
Summary by CodeRabbit
New Features
Breaking Changes
Improvements
Documentation
Chores