[IMPLEMENT] Feature/1 overdue orders#12
[IMPLEMENT] Feature/1 overdue orders#12florinc wants to merge 6 commits intofeature/1-overdue-ordersfrom
Conversation
florinc
left a comment
There was a problem hiding this comment.
Verdict
APPROVE WITH SUGGESTIONS
The implementation is faithful to both design documents with no blocking issues. All architectural boundaries are respected and test coverage is comprehensive. Two minor suggestions for improvement: remove unused import and add missing documentation comment.
Summary
The implementation correctly follows the detailed design specification across all dimensions. The service method implements the exact query logic specified (filtering by date and status, aggregating counts, sorting by oldest due date), the DTO matches the contract precisely, the console command uses the correct pattern, and all 8 required unit tests are present with proper AAA structure and isolation. Architecture compliance is excellent: dependency rules are followed, service registration uses the correct attribute, data access uses IRepository (no DbContext leakage), primary constructors are used, and nullability is enabled. Code quality is high with self-documenting names and no unnecessary comments.
Metrics
| Severity | Count |
|---|---|
| 🔴 Blocker | 0 |
| 🟡 Warning | 0 |
| 🟢 Suggestion | 2 |
| ℹ️ Note | 1 |
| Total | 3 |
There was a problem hiding this comment.
Pull request overview
Implements Sales feature #1 (“overdue orders”) by extending the Sales customer service contract, adding a console command to display customers with overdue orders, and introducing a new unit test project to validate the behavior.
Changes:
- Extend
ICustomerService+ addCustomerOverdueOrdersDataDTO for overdue-order reporting. - Implement
GetCustomersWithOverdueOrders()inSales.Servicesand add a console command to show results. - Add
Sales.Services.UnitTestsproject with coverage for filtering/aggregation/sorting.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
Modules/Sales/Sales.Services/Sales.Services.csproj |
Adds InternalsVisibleTo to enable unit testing (and DynamicProxy visibility). |
Modules/Sales/Sales.Services/CustomerService.cs |
Adds overdue-orders query + logging and updates DI ctor. |
Modules/Sales/Sales.Services.UnitTests/xunit.runner.json |
Adds xUnit runner config for the new test project. |
Modules/Sales/Sales.Services.UnitTests/Sales.Services.UnitTests.csproj |
Introduces the Sales.Services unit test project and dependencies. |
Modules/Sales/Sales.Services.UnitTests/CustomerServiceTests.cs |
Adds unit tests for overdue-orders behavior. |
Modules/Sales/Sales.ConsoleCommands/ShowCustomersWithOverdueOrdersConsoleCommand.cs |
Adds a menu command to display overdue-order customers in the console UI. |
Modules/Contracts/Sales/ICustomerService.cs |
Adds GetCustomersWithOverdueOrders() to the contract. |
Modules/Contracts/Sales/CustomerOverdueOrdersData.cs |
New DTO for overdue-order output. |
AppInfraDemo.sln |
Adds the new test project, plus additional platform configurations. |
.copilot-cli/1-overdue-orders.coder.cli.md |
Adds a Copilot CLI transcript/log. |
You can also share your feedback on Copilot code review. Take the survey.
| [Fact] | ||
| public void GetCustomersWithOverdueOrders_WithOverdueOrders_ReturnsCustomers() | ||
| { | ||
| var customer = CreateCustomer(1, "John", "Doe", "Acme Corp"); | ||
| var overdueOrder = CreateOrder(1, customer, DateTime.Today.AddDays(-5), SalesOrderHeaderStatusValues.InProcess); | ||
| var repositoryStub = new FakeRepository(new[] { overdueOrder }); | ||
| var target = GetTarget(repositoryStub); | ||
|
|
||
| var result = target.GetCustomersWithOverdueOrders(); | ||
|
|
| var results = repository.GetEntities<SalesOrderHeader>() | ||
| .Where(o => o.DueDate < today && !closedStatuses.Contains(o.Status)) | ||
| .GroupBy(o => o.Customer) | ||
| .Select(g => new CustomerOverdueOrdersData | ||
| { | ||
| CustomerName = !string.IsNullOrWhiteSpace(g.Key.CompanyName) | ||
| ? g.Key.CompanyName | ||
| : !string.IsNullOrWhiteSpace(g.Key.FirstName) || !string.IsNullOrWhiteSpace(g.Key.LastName) | ||
| ? $"{g.Key.FirstName} {g.Key.LastName}".Trim() | ||
| : $"Customer {g.Key.CustomerID}", | ||
| OverdueOrderCount = g.Count(), | ||
| OldestOverdueOrderDate = g.Min(o => o.DueDate) |
| <TargetFramework>net10.0</TargetFramework> | ||
| <ImplicitUsings>enable</ImplicitUsings> | ||
| <Nullable>enable</Nullable> | ||
| <OutputType>Exe</OutputType> |
| --- | ||
|
|
||
| florin@Florin_Razer C: cli feature-14 feature/cli/14-overdue-orders copilot --agent=coder ` | ||
| > -p "Use Mode 2 to create Unit test for feature #1 as specified into the detailed design from docs/workitems/1-detailed-design.md | ||
| > Context: Mode=2; Files=Modules/Contracts/Sales/CustomerOverdueOrdersData.cs, Modules/Contracts/Sales/ICustomerService.cs, Modules/Sales/Sales.Services/CustomerService.cs, Modules/Sales/Sales.ConsoleCommands/ShowCustomersWithOverdueOrdersConsoleCommand.cs; Build=PASS; Tests=PASS; Deviations=NONE Commits=03866e1: [AI:Coder, HUMAN:-, MODEL: Claude 3.7 Sonnet] (#1) Add CustomerOverdueOrdersData DTO, extend ICustomerService, and implement GetCustomersWithOverdueOrders, 9754ccb: [AI:Coder, HUMAN:-, MODEL: Claude 3.7 Sonnet] (#1) Add ShowCustomersWithOverdueOrdersConsoleCommand" ` | ||
| > --allow-all-tools |
| Debug|x64 = Debug|x64 | ||
| Debug|x86 = Debug|x86 | ||
| Release|Any CPU = Release|Any CPU | ||
| Release|x64 = Release|x64 | ||
| Release|x86 = Release|x86 | ||
| EndGlobalSection | ||
| GlobalSection(ProjectConfigurationPlatforms) = postSolution | ||
| {E513E557-3FAF-0D2E-F72A-11171988EE60}.Debug|Any CPU.ActiveCfg = Debug|Any CPU | ||
| {E513E557-3FAF-0D2E-F72A-11171988EE60}.Debug|Any CPU.Build.0 = Debug|Any CPU | ||
| {E513E557-3FAF-0D2E-F72A-11171988EE60}.Debug|x64.ActiveCfg = Debug|Any CPU | ||
| {E513E557-3FAF-0D2E-F72A-11171988EE60}.Debug|x64.Build.0 = Debug|Any CPU | ||
| {E513E557-3FAF-0D2E-F72A-11171988EE60}.Debug|x86.ActiveCfg = Debug|Any CPU | ||
| {E513E557-3FAF-0D2E-F72A-11171988EE60}.Debug|x86.Build.0 = Debug|Any CPU | ||
| {E513E557-3FAF-0D2E-F72A-11171988EE60}.Release|Any CPU.ActiveCfg = Release|Any CPU | ||
| {E513E557-3FAF-0D2E-F72A-11171988EE60}.Release|Any CPU.Build.0 = Release|Any CPU | ||
| {E513E557-3FAF-0D2E-F72A-11171988EE60}.Release|x64.ActiveCfg = Release|Any CPU |
|
|
||
| <ItemGroup> | ||
| <InternalsVisibleTo Include="Sales.Services.UnitTests" /> | ||
| <InternalsVisibleTo Include="DynamicProxyGenAssembly2" Key="0024000004800000940000000602000000240000525341310004000001000100c547cac37abd99c8db225ef2f6c8a3602f3b3606cc9891605d02baa56104f4cfc0734aa39b93bf7852f7d9266654753cc297e7d2edfe0bac1cdcf9f717241550e0a7b191195b7667bb4f64bcb8e2121380fd1d9d46ad2d92d2d15605093924cceaf74c4861eff62abf69b9291ed0a340e113be11e6a7d3113e92484cf7045cc7" /> |
… be only as Markdown
338cd5d to
506e877
Compare
…e CLI when executing the model
…eOrdersData DTO, extend ICustomerService, and implement GetCustomersWithOverdueOrders
…ithOverdueOrdersConsoleCommand
… GetCustomersWithOverdueOrders
506e877 to
f4e2d36
Compare
No description provided.