Feature/124 contact lists #143
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Added contact imports management interfaces, models, requests, and validation - Added example project to deminstrate Contact Imports API usage - Updated Contacts Interface to provide access to Contact Imports
- Added Contacts Import Unit tests - Added Contacts Import Integrational tests - Updates in Contacts Import and Contacts models for unification and to support serialization
- Corrected documentation for contact imports and requests
Enhance validation for ContactsImportRequest and improve documentation
- Enhance validation tests for ContactsImportRequest to include edge cases for collection size and entries format - Corrected ContactImportRequest documentation
- Corrected contact import variable names and - updated contact import status property - corrections in contact import integrational tests
– Added Contact Lists management interfaces, models, requests, and validation – Added example project to demonstrate Contact Lists API usage – Updated Contacts interface to provide access to Contact Lists – Added Contact Lists unit tests – Added Contact Lists integration tests – Extended Contacts unit tests to cover new methods for accessing lists and imports Implemented comparer for Contacts in Contacts integration tests for feature #140: now it properly compares the actual results of requests with the expected ones.
- XML Docs corrections - added tests for DateTimeToUnixMsNullableJsonConverter
dr-3lo
requested review from
mklocek,
vittorius and
zhaparoff
as code owners
WalkthroughAdds Contact Lists and import navigation: new abstractions, models, validators, runtime resources, example project, serializer annotations, global usings, and companion unit/integration tests and test helpers. Also adds two example projects to the solution and updates solution nesting and configuration mappings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev
participant ExampleApp as Example App
participant MailtrapClient as MailtrapClient
participant ContactsCol as Contacts Collection
participant ListsCol as Contact Lists Collection
participant ListRes as Contact List Resource
participant API as Mailtrap HTTP API
Dev->>ExampleApp: run
ExampleApp->>MailtrapClient: configure (ApiToken)
ExampleApp->>MailtrapClient: Account(id).Contacts()
MailtrapClient->>ContactsCol: access collection
Note over ContactsCol,ListsCol: New navigation methods
ContactsCol->>ListsCol: Lists()
ListsCol->>API: GET /accounts/{id}/contacts/lists
API-->>ListsCol: 200 [ContactsList[]]
ListsCol-->>ExampleApp: IList<ContactsList>
alt create list
ExampleApp->>ListsCol: Create(request)
ListsCol->>API: POST /accounts/{id}/contacts/lists
API-->>ListsCol: 201 ContactsList
ListsCol-->>ExampleApp: ContactsList
end
ExampleApp->>ContactsCol: List(listId)
ContactsCol->>ListRes: resource for /lists/{listId}
par details / update / delete
ListRes->>API: GET /accounts/{id}/contacts/lists/{listId}
API-->>ListRes: 200 ContactsList
ListRes->>API: PATCH /accounts/{id}/contacts/lists/{listId}
API-->>ListRes: 200 ContactsList
ListRes->>API: DELETE /accounts/{id}/contacts/lists/{listId}
API-->>ListRes: 204 No Content
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 1
🧹 Nitpick comments (48)
tests/Mailtrap.UnitTests/Contacts/Converters/DateTimeToUnixMsNullableJsonConverterTests.cs (3)
29-37: Strengthen assertion: verify JSON number kind, not just string equalityAssert the JSON token type is Number and its value equals expected ms. This makes the test resilient to incidental whitespace/formatting.
Apply:
- // Assert - json.Should().Be(expectedMs.ToString(CultureInfo.InvariantCulture)); + // Assert + using var doc = System.Text.Json.JsonDocument.Parse(json); + var root = doc.RootElement; + root.ValueKind.Should().Be(System.Text.Json.JsonValueKind.Number); + root.GetInt64().Should().Be(expectedMs);
63-70: Reduce brittleness of exception message assertionMessages can drift; assert on content instead of full pattern.
- act.Should().Throw<JsonException>().WithMessage("*Expected number for Unix time milliseconds*"); + act.Should().Throw<JsonException>() + .Which.Message.Should().Contain("Expected number");
1-71: Add a few edge‑case tests (offsets, negatives, non‑integral numbers)Covers important edges without overfitting.
@@ public void Read_InvalidTokenType_ThrowsException() { // Arrange var json = "\"not a number\""; // Act & Assert var act = () => JsonSerializer.Deserialize<DateTimeOffset?>(json, _options); - act.Should().Throw<JsonException>().WithMessage("*Expected number for Unix time milliseconds*"); + act.Should().Throw<JsonException>().Which.Message.Should().Contain("Expected number"); } } + + [Test] + public void Write_LocalOffset_SerializesToUnixMs() + { + // Arrange + var local = new DateTimeOffset(2020, 01, 01, 00, 00, 00, TimeSpan.FromHours(-5)); + var expectedMs = local.ToUnixTimeMilliseconds(); + // Act + var json = JsonSerializer.Serialize<DateTimeOffset?>(local, _options); + // Assert + using var doc = System.Text.Json.JsonDocument.Parse(json); + doc.RootElement.GetInt64().Should().Be(expectedMs); + } + + [Test] + public void Read_NegativeUnixMs_DeserializesToCorrectDate() + { + // Arrange + long ms = -1000; // 1969-12-31T23:59:59Z + var json = ms.ToString(CultureInfo.InvariantCulture); + // Act + var result = JsonSerializer.Deserialize<DateTimeOffset?>(json, _options); + // Assert + result.Should().Be(DateTimeOffset.FromUnixTimeMilliseconds(ms)); + } + + [Test] + public void Read_NonIntegralNumber_ThrowsJsonException() + { + // Arrange: exponent/decimal form should not be accepted by GetInt64 + var json = "1.5778368e12"; + // Act + var act = () => JsonSerializer.Deserialize<DateTimeOffset?>(json, _options); + // Assert + act.Should().Throw<JsonException>(); + }src/Mailtrap.Abstractions/GlobalUsings.cs (1)
30-36: Add missing root namespace for ContactLists (consistency with ContactImports)You added
Mailtrap.ContactImportsroot but notMailtrap.ContactLists. Recommend adding it for parity and fewer fully‑qualified references.global using Mailtrap.ContactImports.Requests; global using Mailtrap.ContactImports.Validators; +global using Mailtrap.ContactLists; global using Mailtrap.ContactLists.Models; global using Mailtrap.ContactLists.Requests; global using Mailtrap.ContactLists.Validators;examples/Mailtrap.Example.ContactLists/appsettings.json (1)
1-17: Keep secrets out of source; prefer User Secrets or env varsGood sample config. Please document using
dotnet user-secretsor environment variables forMailtrap:ApiTokento avoid accidental key commits.src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs (1)
21-24: Parameterless ctor: clarify serializer intent and selectionTo make constructor selection explicit and avoid ambiguity with the
(string email)ctor, consider marking the parameterless ctor with[JsonConstructor]. Also please confirm the baseContactRequestsupports parameterless construction in this PR./// <summary> /// Parameterless instance constructor for serializers. /// </summary> - public CreateContactRequest() { } + [JsonConstructor] + public CreateContactRequest() { }tests/Mailtrap.IntegrationTests/TestExtensions/EmailAddressHelpers.cs (1)
14-49: Consider edge case handling and domain validation.The email generation logic is mostly sound, but there are a few areas for improvement:
Line 39-42: When
outputLengthis less than minimum requirements, returningrandom.GetString(outputLength.Value)produces a random string that's not a valid email format, which may cause test failures.Lines 24-33: No validation is performed on the provided
domainparameter - it could contain invalid characters or format.Line 46: The calculation
usernameLength ?? defaultUsernameLengthcould result in a negative value ifoutputLengthis very small but still >=minEmailLength.Consider these improvements:
// Explicit check for valid outputLength if (outputLength.HasValue && outputLength < minEmailLength) { - return random.GetString(outputLength.Value); + throw new ArgumentException($"Output length {outputLength.Value} is too small for a valid email. Minimum required: {minEmailLength}", nameof(outputLength)); } // Generation of username with optimal length var usernameLength = outputLength - atSymbol.Length - domainPart.Length; +if (usernameLength < 1) +{ + throw new ArgumentException($"Cannot generate valid email with specified output length {outputLength}", nameof(outputLength)); +} var username = random.GetString(usernameLength ?? defaultUsernameLength);tests/Mailtrap.IntegrationTests/TestExtensions/ValidationHelpers.cs (2)
6-17: Good reuse-safety on StringContent stream; minor robustness tweak availableResetting the stream after deserialization is correct to allow reuse. Optionally also seek to 0 before deserialization to be defensive if a reused content was previously read.
var responseStream = await responseContent.ReadAsStreamAsync(); + if (responseStream.CanSeek) + { + responseStream.Position = 0; + } var expectedResponse = await JsonSerializer.DeserializeAsync<TValue>(responseStream, jsonSerializerOptions); if (responseStream.CanSeek) { responseStream.Position = 0; // Reset stream position }
19-40: Tidy up: fix typos, avoid shadowing, and use GetRawText for stable JsonElement comparison
- Fix typos in XML docs/comments.
- Avoid shadowing the method parameter name with a local variable named expected.
- Prefer JsonElement.GetRawText() over ToString() for exact structural equality.
- /// <typeparam name="TValue">Suppose to be <see cref="ContactResponse"/> and derived classes.</typeparam> - /// <param name="result">Object with actial result</param> + /// <typeparam name="TValue">Supposed to be <see cref="ContactResponse"/> and derived classes.</typeparam> + /// <param name="result">Object with actual result</param> /// <param name="expected">Object with expected results</param> internal static void ShouldBeEquivalentToContactResponse<TValue>(this TValue result, TValue expected) where TValue : class { result.Should() .NotBeNull() .And .BeEquivalentTo(expected, options => options // Convert JsonElement to string before comparison - // this should allow to correctly compare Dictionaty<string, object> like Contact.Fields + // this should allow to correctly compare Dictionary<string, object> like Contact.Fields .Using<JsonElement>(ctx => { - var expected = ctx.Expectation.ToString(); - var actual = ctx.Subject.ToString(); - actual.Should().Be(expected); + var expectedText = ctx.Expectation.GetRawText(); + var actualText = ctx.Subject.GetRawText(); + actualText.Should().Be(expectedText); }).WhenTypeIs<JsonElement>()); }tests/Mailtrap.IntegrationTests/Contacts/ContactsIntegrationTests.cs (3)
85-86: Silence nullable warnings at call siteYou already assert not null; use the null‑forgiving operator when passing expectedResponse to the comparer.
-expectedResponse.Should().NotBeNull(); +expectedResponse.Should().NotBeNull(); ... -result.ShouldBeEquivalentToContactResponse(expectedResponse); +result.ShouldBeEquivalentToContactResponse(expectedResponse!);Also applies to: 114-114
165-167: Repeat: apply null‑forgiving for claritySame as above for GetDetails_Success.
-expectedResponse.Should().NotBeNull(); +expectedResponse.Should().NotBeNull(); ... -result.ShouldBeEquivalentToContactResponse(expectedResponse); +result.ShouldBeEquivalentToContactResponse(expectedResponse!);Also applies to: 194-194
205-205: Repeat: apply null‑forgiving for Update_SuccessKeep consistency across tests.
-expectedResponse.Should().NotBeNull(); +expectedResponse.Should().NotBeNull(); ... -result.ShouldBeEquivalentToContactResponse(expectedResponse); +result.ShouldBeEquivalentToContactResponse(expectedResponse!);Also applies to: 209-211, 239-239
examples/Mailtrap.Example.ContactImports/Mailtrap.Example.ContactImports.csproj (1)
3-4: Confirm VersionOverride usage; prefer central versions or explicit Version.If the repo uses Central Package Management (Directory.Packages.props), VersionOverride is fine; otherwise use Version for clarity.
Apply if not using CPM:
- <PackageReference Include="Microsoft.Extensions.Http" VersionOverride="9.0.8" /> - <PackageReference Include="System.Text.Json" VersionOverride="9.0.7" /> + <PackageReference Include="Microsoft.Extensions.Http" Version="9.0.8" /> + <PackageReference Include="System.Text.Json" Version="9.0.7" />tests/Mailtrap.UnitTests/ContactImports/ContactsImportCollectionResourceTests.cs (1)
1-52: Add coverage for Create() behavior.Consider tests verifying Create() delegates to the correct command with the given request and passes the CancellationToken.
tests/Mailtrap.UnitTests/ContactLists/ContactsListResourceTests.cs (1)
1-52: Add behavior tests for GetDetails/Update/Delete.Mock and assert correct command invocation and token flow for each method.
src/Mailtrap/ContactLists/ContactsListResource.cs (1)
6-15: Solid implementation; consider explicit null-guard on Update request.If base Update(...) doesn’t already throw on null request, add a fast ArgumentNullException here to fail early.
Possible tweak:
public async Task<ContactsList> Update(ContactsListRequest request, CancellationToken cancellationToken = default) - => await Update<ContactsListRequest, ContactsList>(request, cancellationToken).ConfigureAwait(false); + { + if (request is null) throw new ArgumentNullException(nameof(request)); + return await Update<ContactsListRequest, ContactsList>(request, cancellationToken).ConfigureAwait(false); + }src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs (1)
27-28: Note: object-typed Fields may deserialize numbers as JsonElement.
If consumers rely on numeric/string fidelity (e.g., zip codes), consider documenting this or offering a typed alternative/converter.examples/Mailtrap.Example.ContactImports/appsettings.json (1)
14-16: Avoid embedding secrets in sample config; prefer env override.
Keep placeholder, but recommend reading MAILTRAP_API_TOKEN from environment with config fallback to avoid accidental commits of real tokens.tests/Mailtrap.UnitTests/ContactLists/Requests/ContactsListRequestTests.cs (2)
8-13: Fix test names: “Email” → “Name”; clarify initialization name.
Method names reference Email but the subject is ContactsListRequest.Name.Apply:
- public void Constructor_ShouldThrowArgumentNullException_WhenEmailIsNull() + public void Constructor_ShouldThrowArgumentNullException_WhenNameIsNull() @@ - public void Constructor_ShouldThrowArgumentNullException_WhenEmailIsEmpty() + public void Constructor_ShouldThrowArgumentNullException_WhenNameIsEmpty() @@ - public void Constructor_ShouldInitializeFieldsCorrectly() + public void Constructor_ShouldInitializeNameCorrectly()Also applies to: 16-21, 24-34
36-48: Add a positive boundary test (length=255).
Complements the failing-case test at 256 to lock the validator’s upper bound.tests/Mailtrap.IntegrationTests/ContactImports/Create_Success_Request.json (1)
6-9: Zip/postal codes should be strings to preserve leading zeros.
Using numbers risks truncation of leading zeros in real data.Apply:
- "zip_code": 11111 + "zip_code": "11111" @@ - "zip_code": 22222 + "zip_code": "22222"Also applies to: 24-27
src/Mailtrap.Abstractions/ContactLists/Validators/ContactsListRequestValidator.cs (1)
20-21: Reject whitespace-only names; keep length rule.
Current NotEmpty allows " ". Add a whitespace check.Apply:
- RuleFor(r => r.Name).NotEmpty().Length(1, 255); + RuleFor(r => r.Name) + .Must(n => !string.IsNullOrWhiteSpace(n)) + .WithMessage("Name must not be empty or whitespace.") + .Length(1, 255);tests/Mailtrap.UnitTests/ContactImports/ContactsImportResourceTests.cs (1)
39-47: Consider a unit test for GetDetails path/command dispatch.
Add a test that verifies the resource issues a GET for the built URI (mock the command factory) and honors CancellationToken.tests/Mailtrap.IntegrationTests/TestConstants/UrlSegmentsTestConstants.cs (1)
21-22: Unify URL segment constants across test projects.
tests/Mailtrap.IntegrationTests/TestConstants/UrlSegmentsTestConstants.cs defines EmailsSegment = "messages" while tests/Mailtrap.UnitTests/TestConstants/UrlSegmentsTestConstants.cs defines MessagesSegment = "messages"; align the identifier names or extract a shared test-constants module to avoid drift.src/Mailtrap/ContactImports/ContactsImportResource.cs (1)
3-5: Nit: tighten summary wording.Consider clarifying this is the single-import resource (details for a specific import), not the collection.
/// <summary> -/// Implementation of Contact Imports API operations. +/// Resource for a specific Contacts Import (fetches details/status). /// </summary>src/Mailtrap.Abstractions/ContactImports/IContactsImportCollectionResource.cs (2)
14-16: Fix misleading XML doc sentence.“List all contacts with details into request” reads wrong for a create/import call.
- /// List all contacts with details into <paramref name="request"/>. + /// Provide the contacts to import in the <paramref name="request"/>.
25-27: Clarify return description.Be explicit that the result is the created import descriptor with ID and initial status.
- /// Contact import id and status. + /// ContactsImport containing the import ID and initial status.tests/Mailtrap.UnitTests/ContactImports/Requests/ContactsImportRequestTests.cs (2)
37-53: Mark the 50,001‑items test as slow to keep default CI runs snappy.This test builds a 50k+ list; flag it to allow selective execution.
- [Test] + [Test, Category("Slow")] public void Validate_ShouldFail_WhenProvidedCollectionSizeIsInvalid([Values(0, 50001)] int size)
95-102: Stabilize “valid email” generation.Avoid rare collisions with special chars from Random.GetString; use a deterministic, always‑valid email.
- private static ContactImportRequest RandomContactImportRequest() - { - var email = TestContext.CurrentContext.Random.GetString(5) - + "@" - + TestContext.CurrentContext.Random.GetString(5) - + ".com"; - return new ContactImportRequest(email); - } + private static ContactImportRequest RandomContactImportRequest() + { + var email = $"{Guid.NewGuid():N}@example.test"; + return new ContactImportRequest(email); + }examples/Mailtrap.Example.ContactLists/Program.cs (2)
26-27: Don’t hardcode account ID; read from config and validate.Improves portability of the sample.
- var accountId = 12345; - IAccountResource accountResource = mailtrapClient.Account(accountId); + var accountId = config.GetValue<long>("AccountId"); + if (accountId <= 0) + { + logger.LogError("Configure 'Mailtrap:AccountId' in appsettings.json or environment."); + Environment.ExitCode = 1; + return; + } + IAccountResource accountResource = mailtrapClient.Account(accountId);
21-23: Thread a CancellationToken from host lifetime through SDK calls.Helps graceful shutdown and mirrors SDK APIs.
-ILogger<Program> logger = host.Services.GetRequiredService<ILogger<Program>>(); -IMailtrapClient mailtrapClient = host.Services.GetRequiredService<IMailtrapClient>(); +ILogger<Program> logger = host.Services.GetRequiredService<ILogger<Program>>(); +IMailtrapClient mailtrapClient = host.Services.GetRequiredService<IMailtrapClient>(); +var lifetime = host.Services.GetRequiredService<IHostApplicationLifetime>(); +var ct = lifetime.ApplicationStopping; @@ - IList<ContactsList> contactsLists = await contactsListsResource.GetAll(); + IList<ContactsList> contactsLists = await contactsListsResource.GetAll(ct); @@ - contactsList = await contactsListsResource.Create(createContactsListRequest); + contactsList = await contactsListsResource.Create(createContactsListRequest, ct); @@ - ContactsList contactsListResponse = await contactsListResource.GetDetails(); + ContactsList contactsListResponse = await contactsListResource.GetDetails(ct); @@ - ContactsList updateContactsListResponse = await contactsListResource.Update(updateContactsListRequest); + ContactsList updateContactsListResponse = await contactsListResource.Update(updateContactsListRequest, ct); @@ - await contactsListResource.Delete(); + await contactsListResource.Delete(ct);Also applies to: 36-37, 46-47, 54-58, 61-63, 67-68
src/Mailtrap.Abstractions/Contacts/Requests/ContactImportRequest.cs (1)
15-18: Doc: capture constraints in XML.Document that IDs must be positive and disjoint; helps consumers and aligns with added validation.
- /// Contact list IDs to include. + /// Contact list IDs to include (positive, unique, and not present in list_ids_excluded). @@ - /// Contact list IDs to exclude. + /// Contact list IDs to exclude (positive, unique, and not present in list_ids_included).Also applies to: 26-29
examples/Mailtrap.Example.ContactImports/Program.cs (4)
64-69: Avoid FailFast; the subsequent throw is unreachableEnvironment.FailFast terminates the process immediately and prevents graceful disposal/flush. The following throw is dead code. Prefer letting the exception propagate or setting an exit code.
catch (Exception ex) { logger.LogError(ex, "An error occurred during API call."); - Environment.FailFast(ex.Message); - throw; + throw; }
28-30: Don’t hardcode accountId; pull from configurationKeeps the sample copy‑pasteable across environments.
- var accountId = 12345; + long accountId = config.GetValue<long>("AccountId");
49-50: Log stable identifiers (Id/Status) instead of the whole objectImproves log usefulness and avoids ToString()-dependent output.
- logger.LogInformation("Created contact import: {Import}", importResponse); + logger.LogInformation("Created contact import: Id={ImportId}, Status={Status}", importResponse.Id, importResponse.Status); - logger.LogInformation("Contacts Import Details: {Details}", contactsImportDetails); + logger.LogInformation("Contacts Import Details: Id={ImportId}, Status={Status}", contactsImportDetails.Id, contactsImportDetails.Status);Also applies to: 56-57
49-49: Optionally pass a CancellationToken from ApplicationStoppingAllows Ctrl+C and host shutdown to cancel in‑flight requests.
+ var ct = host.Services.GetRequiredService<IHostApplicationLifetime>().ApplicationStopping; - ContactsImport importResponse = await contactsImportsResource.Create(importRequest); + ContactsImport importResponse = await contactsImportsResource.Create(importRequest, ct); ... - ContactsImport contactsImportDetails = await contactsImportResource.GetDetails(); + ContactsImport contactsImportDetails = await contactsImportResource.GetDetails(ct);Also applies to: 56-56
src/Mailtrap.Abstractions/ContactLists/Requests/ContactsListRequest.cs (1)
28-29: Doc nit: tighten wordingMinor grammar/clarity.
- /// Contacts List's <paramref name="name"/> must be min 1 characters and max 255 characters long. + /// The <paramref name="name"/> must be between 1 and 255 characters.src/Mailtrap.Abstractions/ContactLists/IContactsListCollectionResource.cs (1)
19-20: Prefer IReadOnlyList in public APIAvoids exposing mutability in return types while remaining source/ABI compatible with List.
- public Task<IList<ContactsList>> GetAll(CancellationToken cancellationToken = default); + public Task<IReadOnlyList<ContactsList>> GetAll(CancellationToken cancellationToken = default);src/Mailtrap.Abstractions/ContactLists/IContactsListResource.cs (1)
50-55: Doc typo: “contact” → “contacts list”.Fix copy-paste in the Delete remarks.
- /// After deletion of the contact, represented by the current resource instance, it will be no longer available.<br /> + /// After deletion of the contacts list, represented by the current resource instance, it will be no longer available.<br />tests/Mailtrap.IntegrationTests/ContactLists/ContactsListIntegrationTests.cs (1)
84-90: Silence nullable warning when using expectedResponse.Dereferencing expectedResponse.Name may warn; assert+bang or extract a non-null local.
- var expectedResponse = await responseContent.DeserializeStringContentAsync<ContactsList>(_jsonSerializerOptions); - expectedResponse.Should().NotBeNull(); - - var request = new ContactsListRequest(expectedResponse.Name); + var expectedResponse = await responseContent.DeserializeStringContentAsync<ContactsList>(_jsonSerializerOptions); + expectedResponse.Should().NotBeNull(); + var expected = expectedResponse!; + + var request = new ContactsListRequest(expected.Name);src/Mailtrap/Contacts/ContactCollectionResource.cs (1)
10-26: Consider centralizing path segments.If a shared UrlSegments exists, add Imports/List constants there to avoid string duplication across resources and tests.
src/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs (1)
3-7: Nit: summary wording.This type is used to start imports; “CRUD operations” is misleading. Consider “Bulk contacts import request object.”
-/// Generic request object for contacts import CRUD operations. +/// Request object for starting a bulk contacts import.src/Mailtrap.Abstractions/ContactImports/Validators/ContactsImportRequestValidator.cs (5)
4-7: Fix inaccurate XML summary.This validator targets ContactsImportRequest and list bounds, not “contact’s email”. Update to reflect scope.
-/// Validator for Create/Update contact requests.<br /> -/// Ensures contact's email is not empty and length is within the allowed range. +/// Validator for ContactsImportRequest. +/// Ensures the Contacts collection size is within allowed bounds and each item is valid.
10-11: Visibility of constants within an internal type.‘public’ members on an internal class can’t escape the assembly; prefer internal/private for clarity.
- public const int MaxContactsPerRequest = 50_000; - public const int MinContactsPerRequest = 1; + internal const int MaxContactsPerRequest = 50_000; + internal const int MinContactsPerRequest = 1;
22-26: Simplify rules and add actionable messages.NotEmpty() + Count >= 1 is redundant. Also, add messages for better UX.
- RuleFor(r => r.Contacts) - .Cascade(CascadeMode.Stop) - .NotEmpty() - .Must(list => list != null && list.Count is >= MinContactsPerRequest and <= MaxContactsPerRequest); + RuleFor(r => r.Contacts) + .Cascade(CascadeMode.Stop) + .NotNull().WithMessage("'Contacts' must not be null.") + .Must(list => list!.Count is >= MinContactsPerRequest and <= MaxContactsPerRequest) + .WithMessage($"'Contacts' must contain between {MinContactsPerRequest} and {MaxContactsPerRequest} items.");
27-30: Per‑item validator usage — LGTM.Delegating to ContactRequestValidator.Instance aligns with the inheritance model noted in prior PRs. Optionally guard enumeration.
- RuleForEach(r => r.Contacts) + RuleForEach(r => r.Contacts) .NotNull() .SetValidator(ContactRequestValidator.Instance); + // .When(r => r.Contacts is { Count: > 0 }) // optional: skip when null/empty
10-11: Confirmed: MaxContactsPerRequest = 50,000 — matches Mailtrap docs; add docs linkVerified via Mailtrap changelog and features/pricing (50,000 contacts per import request).
File: src/Mailtrap.Abstractions/ContactImports/Validators/ContactsImportRequestValidator.cs
Lines: 10-11public const int MaxContactsPerRequest = 50_000; public const int MinContactsPerRequest = 1;Action: Add an inline comment above MaxContactsPerRequest with a link to the Mailtrap docs/changelog.
Mailtrap.sln (1)
174-181: Configs added for Debug/Release — OK.
Example projects exist and ActiveCfg + Build.0 mappings are present for both; if CI shouldn't build examples, remove the Build.0 lines to skip them.- {E7B8C1F2-9A3D-4C2E-8B7A-6D2F3A1E4B5C}.Debug|Any CPU.Build.0 = Debug|Any CPU - {E7B8C1F2-9A3D-4C2E-8B7A-6D2F3A1E4B5C}.Release|Any CPU.Build.0 = Release|Any CPU - {08B23D8C-7AAC-4CE7-830C-4DFF9F296BFB}.Debug|Any CPU.Build.0 = Debug|Any CPU - {08B23D8C-7AAC-4CE7-830C-4DFF9F296BFB}.Release|Any CPU.Build.0 = Release|Any CPU + # Optional: omit Build.0 to exclude examples from CI solution builds
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (62)
Mailtrap.sln(3 hunks)examples/Mailtrap.Example.ContactImports/Mailtrap.Example.ContactImports.csproj(1 hunks)examples/Mailtrap.Example.ContactImports/Program.cs(1 hunks)examples/Mailtrap.Example.ContactImports/Properties/launchSettings.json(1 hunks)examples/Mailtrap.Example.ContactImports/appsettings.json(1 hunks)examples/Mailtrap.Example.ContactLists/Mailtrap.Example.ContactLists.csproj(1 hunks)examples/Mailtrap.Example.ContactLists/Program.cs(1 hunks)examples/Mailtrap.Example.ContactLists/Properties/launchSettings.json(1 hunks)examples/Mailtrap.Example.ContactLists/appsettings.json(1 hunks)src/Mailtrap.Abstractions/ContactImports/IContactsImportCollectionResource.cs(1 hunks)src/Mailtrap.Abstractions/ContactImports/IContactsImportResource.cs(1 hunks)src/Mailtrap.Abstractions/ContactImports/Models/ContactsImport.cs(1 hunks)src/Mailtrap.Abstractions/ContactImports/Models/ContactsImportStatus.cs(1 hunks)src/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs(1 hunks)src/Mailtrap.Abstractions/ContactImports/Validators/ContactsImportRequestValidator.cs(1 hunks)src/Mailtrap.Abstractions/ContactLists/IContactsListCollectionResource.cs(1 hunks)src/Mailtrap.Abstractions/ContactLists/IContactsListResource.cs(1 hunks)src/Mailtrap.Abstractions/ContactLists/Models/ContactsList.cs(1 hunks)src/Mailtrap.Abstractions/ContactLists/Requests/ContactsListRequest.cs(1 hunks)src/Mailtrap.Abstractions/ContactLists/Validators/ContactsListRequestValidator.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Requests/ContactImportRequest.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs(2 hunks)src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs(2 hunks)src/Mailtrap.Abstractions/Core/Extensions/Ensure.cs(4 hunks)src/Mailtrap.Abstractions/GlobalSuppressions.cs(1 hunks)src/Mailtrap.Abstractions/GlobalUsings.cs(1 hunks)src/Mailtrap/ContactImports/ContactsImportCollectionResource.cs(1 hunks)src/Mailtrap/ContactImports/ContactsImportResource.cs(1 hunks)src/Mailtrap/ContactLists/ContactsListCollectionResource.cs(1 hunks)src/Mailtrap/ContactLists/ContactsListResource.cs(1 hunks)src/Mailtrap/Contacts/ContactCollectionResource.cs(1 hunks)src/Mailtrap/GlobalUsings.cs(1 hunks)tests/Mailtrap.IntegrationTests/ContactImports/ContactImportsIntegrationTests.cs(1 hunks)tests/Mailtrap.IntegrationTests/ContactImports/Create_ShouldFailValidation_WhenProvidedCollectionSizeIsInvalid.json(1 hunks)tests/Mailtrap.IntegrationTests/ContactImports/Create_Success_Request.json(1 hunks)tests/Mailtrap.IntegrationTests/ContactImports/Create_Success_Response.json(1 hunks)tests/Mailtrap.IntegrationTests/ContactImports/GetDetails_Success.json(1 hunks)tests/Mailtrap.IntegrationTests/ContactLists/ContactsListIntegrationTests.cs(1 hunks)tests/Mailtrap.IntegrationTests/ContactLists/Create_Success.json(1 hunks)tests/Mailtrap.IntegrationTests/ContactLists/GetAll_Success.json(1 hunks)tests/Mailtrap.IntegrationTests/ContactLists/GetDetails_Success.json(1 hunks)tests/Mailtrap.IntegrationTests/ContactLists/Update_Success.json(1 hunks)tests/Mailtrap.IntegrationTests/Contacts/ContactsIntegrationTests.cs(6 hunks)tests/Mailtrap.IntegrationTests/GlobalUsings.cs(1 hunks)tests/Mailtrap.IntegrationTests/TestConstants/UrlSegmentsTestConstants.cs(1 hunks)tests/Mailtrap.IntegrationTests/TestExtensions/EmailAddressHelpers.cs(1 hunks)tests/Mailtrap.IntegrationTests/TestExtensions/FileHelpers.cs(1 hunks)tests/Mailtrap.IntegrationTests/TestExtensions/ValidationHelpers.cs(1 hunks)tests/Mailtrap.IntegrationTests/TestingMessages/TestingMessagesIntegrationTests.cs(1 hunks)tests/Mailtrap.UnitTests/ContactImports/ContactsImportCollectionResourceTests.cs(1 hunks)tests/Mailtrap.UnitTests/ContactImports/ContactsImportResourceTests.cs(1 hunks)tests/Mailtrap.UnitTests/ContactImports/Requests/ContactsImportRequestTests.cs(1 hunks)tests/Mailtrap.UnitTests/ContactLists/ContactsListCollectionResourceTests.cs(1 hunks)tests/Mailtrap.UnitTests/ContactLists/ContactsListResourceTests.cs(1 hunks)tests/Mailtrap.UnitTests/ContactLists/Requests/ContactsListRequestTests.cs(1 hunks)tests/Mailtrap.UnitTests/Contacts/ContactCollectionResourceTests.cs(1 hunks)tests/Mailtrap.UnitTests/Contacts/ContactResourceTests.cs(1 hunks)tests/Mailtrap.UnitTests/Contacts/Converters/DateTimeToUnixMsNullableJsonConverterTests.cs(1 hunks)tests/Mailtrap.UnitTests/GlobalUsings.cs(1 hunks)tests/Mailtrap.UnitTests/TestConstants/UrlSegmentsTestConstants.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 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.IntegrationTests/ContactImports/Create_ShouldFailValidation_WhenProvidedCollectionSizeIsInvalid.jsontests/Mailtrap.IntegrationTests/ContactImports/Create_Success_Response.jsontests/Mailtrap.IntegrationTests/GlobalUsings.cstests/Mailtrap.IntegrationTests/TestingMessages/TestingMessagesIntegrationTests.cstests/Mailtrap.IntegrationTests/Contacts/ContactsIntegrationTests.cstests/Mailtrap.IntegrationTests/TestExtensions/ValidationHelpers.cstests/Mailtrap.IntegrationTests/ContactLists/ContactsListIntegrationTests.cstests/Mailtrap.IntegrationTests/ContactImports/ContactImportsIntegrationTests.cstests/Mailtrap.UnitTests/Contacts/Converters/DateTimeToUnixMsNullableJsonConverterTests.cs
📚 Learning: 2025-09-04T08:53:06.292Z
Learnt from: vittorius
PR: railsware/mailtrap-dotnet#139
File: src/Mailtrap.Abstractions/Contacts/Models/Contact.cs:67-70
Timestamp: 2025-09-04T08:53:06.292Z
Learning: In the Mailtrap .NET client, DateTimeOffset properties should not use explicit JsonConverter attributes. All models in the codebase (TestingMessage, Inbox, EmailAttachment, etc.) handle DateTimeOffset properties using default JSON serialization without custom converters, and the Contact model should follow this same pattern for consistency.
Applied to files:
examples/Mailtrap.Example.ContactImports/Mailtrap.Example.ContactImports.csprojtests/Mailtrap.IntegrationTests/GlobalUsings.cssrc/Mailtrap.Abstractions/GlobalUsings.cstests/Mailtrap.IntegrationTests/Contacts/ContactsIntegrationTests.cstests/Mailtrap.UnitTests/Contacts/Converters/DateTimeToUnixMsNullableJsonConverterTests.cs
📚 Learning: 2025-09-04T08:53:30.332Z
Learnt from: vittorius
PR: railsware/mailtrap-dotnet#139
File: tests/Mailtrap.IntegrationTests/Contacts/GetDetails_Success.json:15-16
Timestamp: 2025-09-04T08:53:30.332Z
Learning: The DateTimeToTicksJsonConverter in the Mailtrap .NET client was redundant and problematic because System.Text.Json already handles DateTimeOffset serialization natively to ISO 8601 format, and other models in the codebase (TestingMessage, BillingUsagePeriod) successfully use DateTimeOffset properties without custom converters.
Applied to files:
examples/Mailtrap.Example.ContactImports/Mailtrap.Example.ContactImports.csprojtests/Mailtrap.UnitTests/Contacts/Converters/DateTimeToUnixMsNullableJsonConverterTests.cs
📚 Learning: 2025-09-10T14:37:39.872Z
Learnt from: dr-3lo
PR: railsware/mailtrap-dotnet#142
File: src/Mailtrap/Contacts/ContactCollectionResource.cs:18-19
Timestamp: 2025-09-10T14:37:39.872Z
Learning: The Import method in ContactCollectionResource does not need explicit importId validation because this validation is already implemented in the URI model's Append method when handling long parameters.
Applied to files:
src/Mailtrap/Contacts/ContactCollectionResource.cstests/Mailtrap.UnitTests/ContactImports/ContactsImportResourceTests.cstests/Mailtrap.UnitTests/Contacts/ContactCollectionResourceTests.cstests/Mailtrap.UnitTests/ContactImports/ContactsImportCollectionResourceTests.cssrc/Mailtrap/ContactImports/ContactsImportResource.cssrc/Mailtrap.Abstractions/ContactImports/IContactsImportCollectionResource.cstests/Mailtrap.IntegrationTests/ContactImports/ContactImportsIntegrationTests.cssrc/Mailtrap/ContactImports/ContactsImportCollectionResource.cssrc/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cssrc/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs
📚 Learning: 2025-09-04T12:22:07.783Z
Learnt from: dr-3lo
PR: railsware/mailtrap-dotnet#139
File: src/Mailtrap/Contacts/Requests/ContactRequestExtensions.cs:6-9
Timestamp: 2025-09-04T12:22:07.783Z
Learning: In the Mailtrap .NET client codebase, internal extension methods with non-nullable reference types don't require explicit null guards as the compiler provides compile-time safety and call sites are controlled within the codebase.
Applied to files:
tests/Mailtrap.IntegrationTests/GlobalUsings.cstests/Mailtrap.UnitTests/GlobalUsings.cssrc/Mailtrap.Abstractions/GlobalUsings.csexamples/Mailtrap.Example.ContactImports/Program.cssrc/Mailtrap/GlobalUsings.cs
📚 Learning: 2025-09-10T17:01:49.270Z
Learnt from: dr-3lo
PR: railsware/mailtrap-dotnet#142
File: src/Mailtrap.Abstractions/ContactImports/Validators/ContactsImportRequestValidator.cs:27-30
Timestamp: 2025-09-10T17:01:49.270Z
Learning: In ContactsImportRequestValidator, ContactRequestValidator.Instance is used to validate ContactImportRequest items because ContactImportRequest inherits from ContactRequest and doesn't introduce additional validation rules that would require a separate validator.
Applied to files:
src/Mailtrap.Abstractions/ContactLists/Validators/ContactsListRequestValidator.cstests/Mailtrap.UnitTests/ContactImports/Requests/ContactsImportRequestTests.cssrc/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cssrc/Mailtrap.Abstractions/ContactLists/Requests/ContactsListRequest.cssrc/Mailtrap.Abstractions/ContactImports/Validators/ContactsImportRequestValidator.cssrc/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cssrc/Mailtrap.Abstractions/Contacts/Requests/ContactImportRequest.cs
📚 Learning: 2025-09-04T08:53:06.292Z
Learnt from: vittorius
PR: railsware/mailtrap-dotnet#139
File: src/Mailtrap.Abstractions/Contacts/Models/Contact.cs:67-70
Timestamp: 2025-09-04T08:53:06.292Z
Learning: The Mailtrap .NET client uses default JSON serialization for DateTimeOffset properties without explicit JsonConverter attributes, as seen in models like TestingMessage. DateTime fields should not use the DateTimeToTicksJsonConverter since the API returns Unix epoch milliseconds, not .NET ticks.
Applied to files:
tests/Mailtrap.UnitTests/Contacts/Converters/DateTimeToUnixMsNullableJsonConverterTests.cs
🧬 Code graph analysis (33)
tests/Mailtrap.UnitTests/ContactLists/ContactsListResourceTests.cs (2)
tests/Mailtrap.UnitTests/TestConstants/UrlSegmentsTestConstants.cs (1)
UrlSegmentsTestConstants(4-20)src/Mailtrap/ContactLists/ContactsListResource.cs (2)
ContactsListResource(6-16)ContactsListResource(8-9)
src/Mailtrap/Contacts/ContactCollectionResource.cs (5)
src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (4)
IContactsImportCollectionResource(17-17)IContactsImportResource(34-34)IContactsListCollectionResource(43-43)IContactsListResource(60-60)src/Mailtrap/ContactImports/ContactsImportCollectionResource.cs (2)
ContactsImportCollectionResource(6-13)ContactsImportCollectionResource(8-9)src/Mailtrap/ContactImports/ContactsImportResource.cs (2)
ContactsImportResource(6-13)ContactsImportResource(8-9)src/Mailtrap/ContactLists/ContactsListCollectionResource.cs (2)
ContactsListCollectionResource(6-16)ContactsListCollectionResource(8-9)src/Mailtrap/ContactLists/ContactsListResource.cs (2)
ContactsListResource(6-16)ContactsListResource(8-9)
src/Mailtrap/ContactLists/ContactsListResource.cs (2)
src/Mailtrap/Contacts/ContactCollectionResource.cs (3)
IContactsListResource(25-26)Task(28-29)Task(31-32)src/Mailtrap.Abstractions/ContactLists/IContactsListResource.cs (3)
Task(19-19)Task(36-36)Task(56-56)
tests/Mailtrap.UnitTests/ContactImports/ContactsImportResourceTests.cs (4)
tests/Mailtrap.UnitTests/ContactImports/Requests/ContactsImportRequestTests.cs (5)
TestFixture(4-103)Test(7-13)Test(15-21)Test(23-34)Test(37-53)tests/Mailtrap.UnitTests/ContactImports/ContactsImportCollectionResourceTests.cs (1)
TestFixture(4-52)tests/Mailtrap.UnitTests/Contacts/ContactResourceTests.cs (1)
TestFixture(4-51)tests/Mailtrap.UnitTests/TestConstants/UrlSegmentsTestConstants.cs (1)
UrlSegmentsTestConstants(4-20)
tests/Mailtrap.UnitTests/Contacts/ContactCollectionResourceTests.cs (4)
tests/Mailtrap.UnitTests/TestExtensions/ResourceValidator.cs (2)
ResourceValidator(4-16)Validate(6-15)src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (4)
IContactsImportCollectionResource(17-17)IContactsImportResource(34-34)IContactsListCollectionResource(43-43)IContactsListResource(60-60)src/Mailtrap/Contacts/ContactCollectionResource.cs (4)
IContactsImportCollectionResource(16-17)IContactsImportResource(19-20)IContactsListCollectionResource(22-23)IContactsListResource(25-26)tests/Mailtrap.UnitTests/TestConstants/UrlSegmentsTestConstants.cs (1)
UrlSegmentsTestConstants(4-20)
tests/Mailtrap.UnitTests/ContactLists/Requests/ContactsListRequestTests.cs (1)
src/Mailtrap.Abstractions/ContactLists/Requests/ContactsListRequest.cs (2)
ContactsListRequest(34-39)ContactsListRequest(44-44)
src/Mailtrap.Abstractions/ContactLists/Validators/ContactsListRequestValidator.cs (1)
src/Mailtrap.Abstractions/ContactLists/Requests/ContactsListRequest.cs (2)
ContactsListRequest(34-39)ContactsListRequest(44-44)
tests/Mailtrap.UnitTests/ContactImports/ContactsImportCollectionResourceTests.cs (4)
tests/Mailtrap.UnitTests/ContactImports/ContactsImportResourceTests.cs (1)
TestFixture(4-52)tests/Mailtrap.UnitTests/Contacts/ContactResourceTests.cs (1)
TestFixture(4-51)tests/Mailtrap.UnitTests/TestConstants/UrlSegmentsTestConstants.cs (1)
UrlSegmentsTestConstants(4-20)src/Mailtrap/ContactImports/ContactsImportCollectionResource.cs (2)
ContactsImportCollectionResource(6-13)ContactsImportCollectionResource(8-9)
src/Mailtrap.Abstractions/ContactLists/IContactsListCollectionResource.cs (4)
src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (3)
IContactsListCollectionResource(43-43)Task(73-73)Task(90-90)src/Mailtrap/Contacts/ContactCollectionResource.cs (3)
IContactsListCollectionResource(22-23)Task(28-29)Task(31-32)src/Mailtrap/ContactLists/ContactsListCollectionResource.cs (2)
Task(11-12)Task(14-15)src/Mailtrap.Abstractions/ContactLists/Requests/ContactsListRequest.cs (2)
ContactsListRequest(34-39)ContactsListRequest(44-44)
tests/Mailtrap.UnitTests/ContactImports/Requests/ContactsImportRequestTests.cs (2)
src/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs (2)
ContactsImportRequest(39-47)ContactsImportRequest(52-52)src/Mailtrap.Abstractions/Contacts/Requests/ContactImportRequest.cs (2)
ContactImportRequest(31-31)ContactImportRequest(36-36)
src/Mailtrap.Abstractions/Core/Extensions/Ensure.cs (2)
src/Mailtrap.Abstractions/Core/Models/StringEnum.cs (2)
T(66-72)T(86-97)src/Mailtrap/Core/Extensions/HttpContentExtensions.cs (1)
T(28-35)
src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs (2)
src/Mailtrap.Abstractions/Contacts/Requests/ContactImportRequest.cs (2)
ContactImportRequest(31-31)ContactImportRequest(36-36)tests/Mailtrap.UnitTests/ContactImports/Requests/ContactsImportRequestTests.cs (1)
ContactImportRequest(95-102)
tests/Mailtrap.UnitTests/ContactLists/ContactsListCollectionResourceTests.cs (2)
tests/Mailtrap.UnitTests/TestConstants/UrlSegmentsTestConstants.cs (1)
UrlSegmentsTestConstants(4-20)src/Mailtrap/ContactLists/ContactsListCollectionResource.cs (2)
ContactsListCollectionResource(6-16)ContactsListCollectionResource(8-9)
tests/Mailtrap.IntegrationTests/Contacts/ContactsIntegrationTests.cs (4)
tests/Mailtrap.IntegrationTests/TestExtensions/EmailAddressHelpers.cs (1)
NextEmail(14-49)src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs (2)
CreateContactRequest(19-19)CreateContactRequest(24-24)tests/Mailtrap.IntegrationTests/TestExtensions/ValidationHelpers.cs (1)
ShouldBeEquivalentToContactResponse(25-40)src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs (2)
UpdateContactRequest(19-19)UpdateContactRequest(24-24)
examples/Mailtrap.Example.ContactImports/Program.cs (4)
src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (2)
IContactsImportCollectionResource(17-17)IContactsImportResource(34-34)src/Mailtrap/Contacts/ContactCollectionResource.cs (2)
IContactsImportCollectionResource(16-17)IContactsImportResource(19-20)src/Mailtrap.Abstractions/Contacts/Requests/ContactImportRequest.cs (2)
ContactImportRequest(31-31)ContactImportRequest(36-36)src/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs (2)
ContactsImportRequest(39-47)ContactsImportRequest(52-52)
src/Mailtrap.Abstractions/ContactImports/IContactsImportResource.cs (5)
src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (3)
IContactsImportResource(34-34)Task(73-73)Task(90-90)src/Mailtrap/Contacts/ContactCollectionResource.cs (3)
IContactsImportResource(19-20)Task(28-29)Task(31-32)src/Mailtrap.Abstractions/ContactImports/IContactsImportCollectionResource.cs (1)
Task(28-28)src/Mailtrap/ContactImports/ContactsImportCollectionResource.cs (1)
Task(11-12)src/Mailtrap/ContactImports/ContactsImportResource.cs (1)
Task(11-12)
src/Mailtrap.Abstractions/ContactImports/Models/ContactsImportStatus.cs (1)
src/Mailtrap.Abstractions/Core/Models/StringEnum.cs (1)
StringEnum(41-49)
tests/Mailtrap.IntegrationTests/TestExtensions/ValidationHelpers.cs (2)
tests/Mailtrap.IntegrationTests/TestExtensions/FileHelpers.cs (2)
Task(6-14)Task(16-28)src/Mailtrap/Configuration/MailtrapClientOptions.cs (1)
JsonSerializerOptions(148-154)
src/Mailtrap.Abstractions/ContactLists/IContactsListResource.cs (3)
src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (3)
IContactsListResource(60-60)Task(73-73)Task(90-90)src/Mailtrap/Contacts/ContactCollectionResource.cs (3)
IContactsListResource(25-26)Task(28-29)Task(31-32)src/Mailtrap/ContactLists/ContactsListResource.cs (3)
Task(10-11)Task(12-13)Task(14-15)
tests/Mailtrap.IntegrationTests/ContactLists/ContactsListIntegrationTests.cs (16)
tests/Mailtrap.IntegrationTests/ContactImports/ContactImportsIntegrationTests.cs (5)
Test(33-77)Test(79-122)Test(124-167)Test(169-212)Test(214-253)tests/Mailtrap.IntegrationTests/Contacts/ContactsIntegrationTests.cs (7)
Test(33-71)Test(73-115)Test(117-154)Test(156-195)Test(197-240)Test(242-280)Test(282-315)src/Mailtrap/Configuration/MailtrapClientOptions.cs (1)
JsonSerializerOptions(148-154)tests/Mailtrap.IntegrationTests/TestConstants/UrlSegmentsTestConstants.cs (1)
UrlSegmentsTestConstants(4-23)src/Mailtrap.Abstractions/ContactLists/IContactsListCollectionResource.cs (2)
Task(19-19)Task(36-36)src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (2)
Task(73-73)Task(90-90)tests/Mailtrap.IntegrationTests/TestExtensions/ValidationHelpers.cs (1)
Task(6-17)src/Mailtrap.Abstractions/ContactLists/IContactsListResource.cs (3)
Task(19-19)Task(36-36)Task(56-56)src/Mailtrap/ContactLists/ContactsListCollectionResource.cs (2)
Task(11-12)Task(14-15)src/Mailtrap/ContactLists/ContactsListResource.cs (3)
Task(10-11)Task(12-13)Task(14-15)src/Mailtrap/Contacts/ContactCollectionResource.cs (2)
Task(28-29)Task(31-32)tests/Mailtrap.IntegrationTests/TestExtensions/FileHelpers.cs (1)
Task(6-14)src/Mailtrap/Core/Constants/MimeTypes.cs (2)
MimeTypes(4-19)Application(6-11)src/Mailtrap/Core/Constants/HeaderValues.cs (1)
HeaderValues(7-12)src/Mailtrap.Abstractions/ContactLists/Requests/ContactsListRequest.cs (2)
ContactsListRequest(34-39)ContactsListRequest(44-44)src/Mailtrap/Core/Extensions/HttpMethodEx.cs (1)
HttpMethodEx(7-10)
src/Mailtrap/ContactImports/ContactsImportResource.cs (3)
src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (3)
IContactsImportResource(34-34)Task(73-73)Task(90-90)src/Mailtrap/Contacts/ContactCollectionResource.cs (3)
IContactsImportResource(19-20)Task(28-29)Task(31-32)src/Mailtrap.Abstractions/ContactImports/IContactsImportResource.cs (1)
Task(19-19)
src/Mailtrap.Abstractions/ContactImports/IContactsImportCollectionResource.cs (4)
src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (3)
IContactsImportCollectionResource(17-17)Task(73-73)Task(90-90)src/Mailtrap/Contacts/ContactCollectionResource.cs (3)
IContactsImportCollectionResource(16-17)Task(28-29)Task(31-32)src/Mailtrap/ContactImports/ContactsImportCollectionResource.cs (1)
Task(11-12)src/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs (2)
ContactsImportRequest(39-47)ContactsImportRequest(52-52)
tests/Mailtrap.IntegrationTests/ContactImports/ContactImportsIntegrationTests.cs (9)
tests/Mailtrap.UnitTests/Contacts/ContactCollectionResourceTests.cs (1)
TestFixture(4-143)tests/Mailtrap.UnitTests/ContactImports/ContactsImportCollectionResourceTests.cs (4)
TestFixture(4-52)Test(19-27)Test(29-37)Test(39-47)src/Mailtrap/Configuration/MailtrapClientOptions.cs (1)
JsonSerializerOptions(148-154)tests/Mailtrap.IntegrationTests/TestConstants/UrlSegmentsTestConstants.cs (1)
UrlSegmentsTestConstants(4-23)tests/Mailtrap.IntegrationTests/TestExtensions/ValidationHelpers.cs (1)
Task(6-17)tests/Mailtrap.IntegrationTests/TestExtensions/FileHelpers.cs (2)
Task(6-14)Task(16-28)src/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs (2)
ContactsImportRequest(39-47)ContactsImportRequest(52-52)src/Mailtrap/Core/Constants/MimeTypes.cs (2)
MimeTypes(4-19)Application(6-11)src/Mailtrap/Core/Constants/HeaderValues.cs (1)
HeaderValues(7-12)
src/Mailtrap/ContactImports/ContactsImportCollectionResource.cs (5)
tests/Mailtrap.UnitTests/ContactImports/ContactsImportCollectionResourceTests.cs (1)
ContactsImportCollectionResource(51-51)src/Mailtrap/Contacts/ContactCollectionResource.cs (3)
IContactsImportCollectionResource(16-17)Task(28-29)Task(31-32)src/Mailtrap.Abstractions/ContactImports/IContactsImportCollectionResource.cs (1)
Task(28-28)src/Mailtrap/ContactImports/ContactsImportResource.cs (1)
Task(11-12)src/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs (2)
ContactsImportRequest(39-47)ContactsImportRequest(52-52)
examples/Mailtrap.Example.ContactLists/Program.cs (3)
src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (2)
IContactsListCollectionResource(43-43)IContactsListResource(60-60)src/Mailtrap/Contacts/ContactCollectionResource.cs (2)
IContactsListCollectionResource(22-23)IContactsListResource(25-26)src/Mailtrap.Abstractions/ContactLists/Requests/ContactsListRequest.cs (2)
ContactsListRequest(34-39)ContactsListRequest(44-44)
src/Mailtrap.Abstractions/ContactLists/Requests/ContactsListRequest.cs (2)
src/Mailtrap.Abstractions/Core/Extensions/Ensure.cs (1)
Ensure(9-106)src/Mailtrap.Abstractions/ContactLists/Validators/ContactsListRequestValidator.cs (2)
ContactsListRequestValidator(8-22)ContactsListRequestValidator(18-21)
src/Mailtrap/ContactLists/ContactsListCollectionResource.cs (5)
tests/Mailtrap.UnitTests/ContactLists/ContactsListCollectionResourceTests.cs (1)
ContactsListCollectionResource(51-51)src/Mailtrap/Contacts/ContactCollectionResource.cs (3)
IContactsListCollectionResource(22-23)Task(28-29)Task(31-32)src/Mailtrap.Abstractions/ContactLists/IContactsListCollectionResource.cs (2)
Task(19-19)Task(36-36)src/Mailtrap/ContactLists/ContactsListResource.cs (3)
Task(10-11)Task(12-13)Task(14-15)src/Mailtrap.Abstractions/ContactLists/Requests/ContactsListRequest.cs (2)
ContactsListRequest(34-39)ContactsListRequest(44-44)
src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (3)
src/Mailtrap/Accounts/AccountResource.cs (1)
IContactCollectionResource(56-57)src/Mailtrap.Abstractions/Accounts/IAccountResource.cs (1)
IContactCollectionResource(143-143)src/Mailtrap/Contacts/ContactCollectionResource.cs (4)
IContactsImportCollectionResource(16-17)IContactsImportResource(19-20)IContactsListCollectionResource(22-23)IContactsListResource(25-26)
tests/Mailtrap.UnitTests/Contacts/Converters/DateTimeToUnixMsNullableJsonConverterTests.cs (2)
src/Mailtrap/Configuration/MailtrapClientOptions.cs (1)
JsonSerializerOptions(148-154)src/Mailtrap.Abstractions/Contacts/Converters/DateTimeToUnixMsJsonConverter.cs (2)
DateTimeToUnixMsNullableJsonConverter(6-34)DateTimeOffset(8-22)
src/Mailtrap.Abstractions/ContactImports/Validators/ContactsImportRequestValidator.cs (1)
src/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs (2)
ContactsImportRequest(39-47)ContactsImportRequest(52-52)
src/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs (5)
src/Mailtrap.Abstractions/Contacts/Requests/ContactImportRequest.cs (2)
ContactImportRequest(31-31)ContactImportRequest(36-36)tests/Mailtrap.UnitTests/ContactImports/Requests/ContactsImportRequestTests.cs (1)
ContactImportRequest(95-102)src/Mailtrap.Abstractions/Core/Extensions/Ensure.cs (3)
Ensure(9-106)NotNullOrEmpty(42-57)NotNullOrEmpty(66-81)src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs (1)
ValidationResult(59-64)src/Mailtrap.Abstractions/ContactImports/Validators/ContactsImportRequestValidator.cs (2)
ContactsImportRequestValidator(8-31)ContactsImportRequestValidator(20-30)
src/Mailtrap.Abstractions/Contacts/Requests/ContactImportRequest.cs (2)
tests/Mailtrap.UnitTests/ContactImports/Requests/ContactsImportRequestTests.cs (1)
ContactImportRequest(95-102)src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs (2)
ContactRequest(45-50)ContactRequest(55-55)
tests/Mailtrap.IntegrationTests/TestExtensions/FileHelpers.cs (4)
tests/Mailtrap.IntegrationTests/TestExtensions/ValidationHelpers.cs (1)
Task(6-17)src/Mailtrap.Abstractions/Core/Extensions/Ensure.cs (1)
Ensure(9-106)tests/Mailtrap.IntegrationTests/ContactImports/ContactImportsIntegrationTests.cs (5)
Test(33-77)Test(79-122)Test(124-167)Test(169-212)Test(214-253)tests/Mailtrap.IntegrationTests/Contacts/ContactsIntegrationTests.cs (7)
Test(33-71)Test(73-115)Test(117-154)Test(156-195)Test(197-240)Test(242-280)Test(282-315)
src/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs (2)
23-26: Clarify remark on System.Text.Json behavior for object valuesTighten the docs so consumers know exactly what to expect from object-typed values in Fields.
Apply this doc-only tweak:
- /// <remarks> - /// object-typed values may deserialize numbers as JsonElement - /// </remarks> + /// <remarks> + /// With System.Text.Json, object-typed values deserialize as: + /// numbers/objects/arrays -> JsonElement; strings -> string; booleans -> bool; null -> null. + /// Consumers should handle JsonElement when reading numeric or composite values. + /// </remarks>
59-60: Decide whether deserialization should enforce constructor invariants — repo currently uses paramless [JsonConstructor]Found [JsonConstructor] on parameterless ctors in:
src/Mailtrap.Abstractions/Contacts/Requests/ContactImportRequest.cs,
src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs,
src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs,
src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs,
src/Mailtrap.Abstractions/ContactLists/Requests/ContactsListRequest.cs,
src/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs.Action: keep the attribute on the parameterless ctors (validation deferred to validators) or move [JsonConstructor] to the parameterized ctor in ContactRequest and make the same change across the other request types to preserve consistency and enable constructor-level invariants (e.g., non-empty Email).
tests/Mailtrap.UnitTests/ContactLists/Requests/ContactsListRequestTests.cs (3)
1-1: Strip BOM/invisible char before namespace.There's a leading BOM/zero-width char before the namespace. Remove to avoid noisy diffs and tooling hiccups.
23-34: Prefer deterministic data in unit tests.Random strings can introduce flakiness with future constraints. Use a fixed string.
Apply this diff:
- var name = TestContext.CurrentContext.Random.GetString(5); + var name = new string('a', 5);
36-48: Strengthen validation test and simplify parameterization.Use TestCase over Values here, avoid randomness, and add positive boundary tests (1 and 255) to lock invariants.
Apply this diff:
- [Test] - public void Validate_ShouldFail_WhenProvidedNameLengthIsInvalid([Values(256)] int length) + [TestCase(256)] + public void Validate_ShouldFail_WhenProvidedNameLengthIsInvalid(int length) @@ - var name = TestContext.CurrentContext.Random.GetString(length); + var name = new string('a', length);Add these tests:
[TestCase(1)] [TestCase(255)] public void Validate_ShouldPass_WhenProvidedNameLengthIsValid(int length) { var name = new string('a', length); var request = new ContactsListRequest(name); var result = request.Validate(); result.IsValid.Should().BeTrue(); }Optional: if MailtrapValidationResult exposes error members, assert it flags Name length:
// Example; adjust to your result shape result.Errors.Should().ContainSingle(e => e.PropertyName == nameof(ContactsListRequest.Name));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/Mailtrap.Abstractions/ContactImports/IContactsImportCollectionResource.cs(2 hunks)src/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs(3 hunks)src/Mailtrap.Abstractions/ContactImports/Validators/ContactsImportRequestValidator.cs(1 hunks)src/Mailtrap.Abstractions/ContactLists/IContactsListResource.cs(1 hunks)src/Mailtrap.Abstractions/ContactLists/Requests/ContactsListRequest.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Requests/ContactImportRequest.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs(2 hunks)src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs(1 hunks)tests/Mailtrap.IntegrationTests/TestExtensions/ValidationHelpers.cs(1 hunks)tests/Mailtrap.UnitTests/ContactLists/Requests/ContactsListRequestTests.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- tests/Mailtrap.IntegrationTests/TestExtensions/ValidationHelpers.cs
- src/Mailtrap.Abstractions/ContactLists/IContactsListResource.cs
- src/Mailtrap.Abstractions/ContactImports/Validators/ContactsImportRequestValidator.cs
- src/Mailtrap.Abstractions/ContactLists/Requests/ContactsListRequest.cs
- src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs
- src/Mailtrap.Abstractions/Contacts/Requests/ContactImportRequest.cs
- src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs
- src/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs
- src/Mailtrap.Abstractions/ContactImports/IContactsImportCollectionResource.cs
🔇 Additional comments (3)
tests/Mailtrap.UnitTests/ContactLists/Requests/ContactsListRequestTests.cs (3)
7-13: LGTM: null-argument guard test.This accurately asserts constructor null handling.
5-6: Keep the fixture internal — match repo convention.
The codebase declares test fixtures as internal sealed across tests/, so changing this single class to public would be inconsistent.Likely an incorrect or invalid review comment.
15-21: Keep ArgumentNullException — Ensure.NotNullOrEmpty intentionally throws ArgumentNullException for empty strings.Ensure.NotNullOrEmpty (src/Mailtrap.Abstractions/Core/Extensions/Ensure.cs) and its unit tests (tests/Mailtrap.UnitTests/Core/Extensions/EnsureTests.cs) expect/throw ArgumentNullException for both null and empty; leave the test as-is.
Likely an incorrect or invalid review comment.
vittorius
approved these changes
Sep 16, 2025
| /// The import process runs asynchronously - use the returned | ||
| /// import ID to check the status and results. | ||
| /// List all contacts with details into <paramref name="request"/>. | ||
| /// Provide the contacts to import in the <paramref name="request"/>. |
# Conflicts: # Mailtrap.sln # src/Mailtrap.Abstractions/ContactImports/IContactsImportCollectionResource.cs # src/Mailtrap.Abstractions/ContactImports/Requests/ContactsImportRequest.cs # src/Mailtrap.Abstractions/ContactImports/Validators/ContactsImportRequestValidator.cs # src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs # src/Mailtrap.Abstractions/Contacts/Requests/ContactImportRequest.cs # src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs # src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs # src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs # src/Mailtrap.Abstractions/GlobalUsings.cs # src/Mailtrap/Contacts/ContactCollectionResource.cs # src/Mailtrap/GlobalUsings.cs # tests/Mailtrap.IntegrationTests/ContactImports/ContactImportsIntegrationTests.cs # tests/Mailtrap.IntegrationTests/GlobalUsings.cs # tests/Mailtrap.IntegrationTests/TestConstants/UrlSegmentsTestConstants.cs # tests/Mailtrap.IntegrationTests/TestExtensions/EmailAddressHelpers.cs # tests/Mailtrap.UnitTests/GlobalUsings.cs # tests/Mailtrap.UnitTests/TestConstants/UrlSegmentsTestConstants.cs
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/Mailtrap.Example.ContactImports/Mailtrap.Example.ContactImports.csproj (1)
4-4: Remove redundant System.Text.Json PackageReference from the example projectProject inherits net9.0 (build/mailtrap-example.props) and the repo centrally pins System.Text.Json (Directory.Packages.props → 9.0.9). Remove the per-project reference in examples/Mailtrap.Example.ContactImports/Mailtrap.Example.ContactImports.csproj.
<ItemGroup> - <PackageReference Include="System.Text.Json" /> </ItemGroup>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/Mailtrap.Example.ContactImports/Mailtrap.Example.ContactImports.csproj(1 hunks)examples/Mailtrap.Example.ContactLists/Mailtrap.Example.ContactLists.csproj(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/Mailtrap.Example.ContactLists/Mailtrap.Example.ContactLists.csproj
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-04T08:53:06.292Z
Learnt from: vittorius
PR: railsware/mailtrap-dotnet#139
File: src/Mailtrap.Abstractions/Contacts/Models/Contact.cs:67-70
Timestamp: 2025-09-04T08:53:06.292Z
Learning: In the Mailtrap .NET client, DateTimeOffset properties should not use explicit JsonConverter attributes. All models in the codebase (TestingMessage, Inbox, EmailAttachment, etc.) handle DateTimeOffset properties using default JSON serialization without custom converters, and the Contact model should follow this same pattern for consistency.
Applied to files:
examples/Mailtrap.Example.ContactImports/Mailtrap.Example.ContactImports.csproj
🔇 Additional comments (1)
examples/Mailtrap.Example.ContactImports/Mailtrap.Example.ContactImports.csproj (1)
3-4: Central package management configured — no action needed.Directory.Packages.props defines PackageVersion entries for Microsoft.Extensions.Http (line 9, Version="9.0.9") and System.Text.Json (line 14, Version="9.0.9"), so the PackageReference entries in examples/Mailtrap.Example.ContactImports/Mailtrap.Example.ContactImports.csproj may omit Version.
This was referenced Sep 19, 2025
Merged
This was referenced Oct 20, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Contacts Import API support into .NET client to resolve #124
Changes
Also
How to test
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests