refactor/clean architecture by alysonsz · Pull Request #3 · alysonsz/Finance-API

alysonsz · 2026-04-07T13:43:42Z

No description provided.

- Add SeedWork (Entity, IAggregateRoot, Result<T>, Unit) - Transform Category to Rich Domain Model with factory method - Transform Transaction to Rich Domain Model with validation logic - Transform User to Rich Domain Model with factory method - Move ETransactionType enum to Domain.Enums - Update TransactionDto to use string for Type property

- Add CategoryDto, TransactionDto, UserDto to Contracts.DTOs - Add ETransactionType enum to Contracts.Enums - Remove DTOs from Domain.Models.DTOs namespace

- Add repository interfaces (ICategoryRepository, ITransactionRepository, IUserRepository) - Implement CQRS handlers for Categories (Create, Update, Delete, GetAll, GetById) - Implement CQRS handlers for Transactions (Create, Update, Delete, GetById, GetByPeriod, GetReport) - Implement CQRS handlers for Auth (Login, Register, GetProfile, UpdateProfile, RefreshToken) - Add TransactionMapper for DTO conversions - Remove old service registrations

- Update CategoryRepository to use Application.Interfaces - Update TransactionRepository to use Application.Interfaces - Update UserRepository to use Application.Interfaces

- Update AuthController to use MediatR commands - Update CategoriesController to use MediatR commands - Update TransactionsController to use MediatR commands - Update BuilderExtension to register repositories and remove services

- Update AuthHandlerTests to use factory methods and reflection - Update CategoryHandlerTests for Rich Domain Models - Update TransactionHandlerTests for Rich Domain Models - Update CategoryRepositoryTests with factory methods - Update TransactionRepositoryTests with factory methods - Update UserRepositoryTests with factory methods - Update CustomWebApplicationFactory with service registrations

- Delete CategoryService, TransactionService, UserService - Delete ICategoryService, ITransactionService, IUserService interfaces

- Update project structure to reflect Clean Architecture - Add Finance.Domain.Tests and separate test projects

Copilot

Pull request overview

This PR refactors the solution toward a stricter Clean Architecture approach by moving repositories to Finance.Application.Interfaces.Repositories, introducing richer domain models (factory methods + update methods with validation), and simplifying the API layer to call MediatR commands directly (removing service wrappers).

Changes:

Introduces domain SeedWork primitives (Entity, IAggregateRoot, Result<T>, Unit) and refactors User, Category, and Transaction to enforce invariants via factory/update methods.
Moves repository interfaces into Finance.Application and updates handlers/controllers/tests to use MediatR + the new rich domain APIs.
Reshapes transaction/category DTOs toward Finance.Contracts.DTOs and updates mapping logic accordingly.

Reviewed changes

Copilot reviewed 61 out of 61 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
README.md	Updates documented project structure to reflect the new layered test projects and seedwork.
Finance.Infrastructure/Repositories/{User,Transaction,Category}Repository.cs	Switches repository interface namespace to `Finance.Application.Interfaces.Repositories`.
Finance.Infrastructure.Tests/Repositories/{User,Transaction,Category}RepositoryTests.cs	Updates test entity creation to use domain factories; introduces reflection-based mutation for updates.
Finance.Domain/SeedWork/{Entity,IAggregateRoot,Result}.cs	Adds seedwork base types and a `Result<T>` pattern used by domain factories/updates.
Finance.Domain/Models/{User,Category,Transaction}.cs	Refactors domain entities into richer models with validation and controlled mutation.
Finance.Domain/Models/DTOs/TransactionDto.cs	Changes `Type` representation to string in the domain DTO.
Finance.Contracts/DTOs/CategoryDto.cs	Adds Contracts DTOs (but includes `TransactionDto` in the same file).
Finance.Contracts/Enums/ETransactionType.cs	Introduces a Contracts enum duplicating the Domain enum.
Finance.Contracts/Interfaces/Services/*.cs	Removes service interfaces that were previously used by API controllers.
Finance.Application/Interfaces/Repositories/*.cs	Adds repository interfaces under Application layer.
Finance.Application/Mappers/TransactionMapper.cs	Updates mapping to the Contracts DTO shape.
Finance.Application/Features/**	Updates handlers to use new repositories, rich domain methods, and `Response<T>.Success/Fail` patterns.
Finance.Api/Extensions/BuilderExtension.cs	Removes `AddServices()` call from startup pipeline while leaving `AddServices()` defined.
Finance.Api/Controllers/{Auth,Categories,Transactions}Controller.cs	Refactors controllers to construct/send MediatR commands directly instead of calling services.
Finance.Api.Tests/CustomWebApplicationFactory.cs	Adds explicit service/repository registrations for test host DI.
.github/workflows/dotnet.yml	Runs CI on pushes to the refactor branch as well as `main`.

Comments suppressed due to low confidence (2)

Finance.Api/Extensions/BuilderExtension.cs:81

AddApiInfrastructure no longer calls AddServices(), but Program.cs only invokes builder.AddApiInfrastructure(). This means repositories, token/cache services, validators, and the outbox hosted service registered in AddServices won't be added to DI, leading to runtime failures when handlers are resolved. Call builder.AddServices() from AddApiInfrastructure (or move these registrations into AddApiInfrastructure).

    public static WebApplicationBuilder AddApiInfrastructure(this WebApplicationBuilder builder)
    {
        builder.AddConfiguration();
        builder.AddDatabase();
        builder.AddCache();
        builder.AddCors();
        builder.AddDocumentation();
        builder.AddMediatR();

        builder.Services.AddHttpContextAccessor();
        builder.Services.AddFluentValidationAutoValidation();
        builder.Services.AddControllers();
        builder.Services.AddJwtAuthentication(builder.Configuration);

        return builder;

Finance.Infrastructure.Tests/Repositories/TransactionRepositoryTests.cs:112

This test updates originalTransaction.Amount via reflection to a positive value while the transaction type is Withdraw. That bypasses the domain normalization rules (withdraw amounts are stored negative) and makes the test assert a state that the domain API would never produce. Prefer using transaction.Update(...) (or creating a new valid transaction) so repository update tests stay consistent with the domain model invariants.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-07T13:51:16Z

    [Authorize]
    [HttpPut("profile")]
    public async Task<IActionResult> UpdateProfileAsync([FromBody] UpdateUserProfileRequest request)
    {
-        var response = await _userService.UpdateProfileAsync(request);
+        var command = new UpdateProfileCommand
+        {
+            Name = request.Name
+        };
+
+        var response = await mediator.Send(command);
        return this.FromResponse(response);


UpdateProfileAsync receives UpdateUserProfileRequest with both Name and Email, and UpdateProfileCommand also has an Email property, but the controller only populates Name. This effectively ignores the request email and will pass an empty Email if the handler ever starts using it. Map request.Email into the command (or remove Email from the request/command if email updates are intentionally unsupported).

Copilot · 2026-04-07T13:51:17Z

        }

-        user.Name = request.Name;
+        var updateResult = user.UpdateProfile(request.Name, user.Email);


The handler calls user.UpdateProfile(request.Name, user.Email), which ignores request.Email (even though UpdateProfileCommand contains Email). As a result, profile updates cannot change email and the command field is unused. Pass request.Email into UpdateProfile (and keep using the domain validation) so the update matches the API contract.

Suggested change

var updateResult = user.UpdateProfile(request.Name, user.Email);

var updateResult = user.UpdateProfile(request.Name, request.Email);

Copilot · 2026-04-07T13:51:17Z

    public static TransactionDto ToDto(Transaction transaction, Category category)
        => new()
        {
            Id = transaction.Id,
            Title = transaction.Title,
            Amount = transaction.Amount,
-            Type = transaction.Type,
+            Type = transaction.Type.ToString(),
            PaidOrReceivedAt = transaction.PaidOrReceivedAt,
            CreatedAt = transaction.CreatedAt,
-            Category = new CategoryDto
-            {
-                Id = category.Id,
-                Title = category.Title,
-                Description = category.Description
-            }
+            CategoryTitle = category.Title
        };


TransactionDto in Finance.Contracts.DTOs includes CategoryId, but TransactionMapper.ToDto doesn't populate it. This will return the default value (0) in API responses, breaking consumers that rely on category id. Map CategoryId = transaction.CategoryId (or category.Id) in the mapper.

Copilot · 2026-04-07T13:51:17Z

        var user = await SeedUserAsync(writeContext);
        var category = await SeedCategoryAsync(writeContext, user.Id);
        var repository = new TransactionRepository(readContext, writeContext);
-        var newTransaction = new Transaction 
-        { 
-            Title = "Almoço", 
-            Amount = 50, 
-            Type = ETransactionType.Withdraw, 
-            UserId = user.Id, 
-            CategoryId = category.Id, 
-            PaidOrReceivedAt = DateTime.UtcNow 
-        };
+        var txResult = Transaction.Create("Almoço", 50, ETransactionType.Withdraw, user.Id, category.Id, DateTime.UtcNow);
+        var newTransaction = txResult.Value;



Transaction.Create expects parameters in the order (title, amount, type, categoryId, userId, paidOrReceivedAt), but this test passes user.Id where categoryId should be and category.Id where userId should be. Swap these arguments so the created transaction has the correct foreign keys (and the test doesn't accidentally rely on both IDs being the same).

Copilot · 2026-04-07T13:51:17Z

        var user = await SeedUserAsync(writeContext);
        var category = await SeedCategoryAsync(writeContext, user.Id);
-        var originalTransaction = new Transaction 
-        { 
-            Title = "Original", 
-            Amount = 100, 
-            PaidOrReceivedAt = DateTime.UtcNow, 
-            UserId = user.Id, 
-            CategoryId = category.Id, 
-            Type = ETransactionType.Withdraw 
-        };
+        var txResult = Transaction.Create("Original", 100, ETransactionType.Withdraw, user.Id, category.Id, DateTime.UtcNow);
+        var originalTransaction = txResult.Value;



Same issue as above: Transaction.Create argument order is (categoryId, userId), but this call passes (user.Id, category.Id). This can persist a transaction linked to the wrong user/category (or fail if IDs differ).

Copilot · 2026-04-07T13:51:19Z

+
+public class TransactionDto
+{
+    public long Id { get; set; }
+    public string Title { get; set; } = string.Empty;
+    public DateTime CreatedAt { get; set; }
+    public DateTime? PaidOrReceivedAt { get; set; }
+    public string Type { get; set; } = string.Empty;
+    public decimal Amount { get; set; }
+    public long CategoryId { get; set; }
+    public string CategoryTitle { get; set; } = string.Empty;
+}


This file defines both CategoryDto and TransactionDto even though it's named CategoryDto.cs. Keeping multiple unrelated DTOs in a single file makes navigation and maintenance harder (and complicates type discovery in IDEs). Split TransactionDto into its own file (e.g., TransactionDto.cs).

Copilot · 2026-04-07T13:51:19Z

+namespace Finance.Contracts.Enums;
+
+public enum ETransactionType
+{
+    Deposit = 1,
+    Withdraw = 2
+}


A new Finance.Contracts.Enums.ETransactionType is introduced, but transaction requests still reference Finance.Domain.Enums.ETransactionType. This duplication can lead to accidental mixing of enums across layers and extra mapping work. Either switch contract types to use the Contracts enum consistently, or remove this enum until the migration is complete.

Copilot · 2026-04-07T13:51:19Z

+├── 📁 Finance.Contracts           # DTOs, Requests, Responses compartilhados
+├── 📁 Finance.Infrastructure      # EF Core, Redis, Repositórios, Services
+├── 📁 Finance.Infrastructure.Tests # Testes de integração de repositories
+├── 📁 Finance.Api                 # Controllers, DI, Middlewares, Program
+└── 📁 Finance.Api.Tests          # Testes de integração da API (Testcontainers)


Minor formatting: the spacing before # is inconsistent on the last two entries (Finance.Infrastructure.Tests and Finance.Api.Tests), which makes the tree harder to scan. Align the padding so the comments column is consistent.

Copilot · 2026-04-07T13:51:20Z

        writeContext.Entry(originalUser).State = EntityState.Detached;

        var repository = new UserRepository(readContext, writeContext);
-        originalUser.Name = "Updated Name";
+        typeof(User).GetProperty("Name")?.SetValue(originalUser, "Updated Name");

        await repository.UpdateAsync(originalUser);


This test mutates User.Name via reflection to bypass the new encapsulation. Since the domain now exposes UpdateProfile(...) (with validation/normalization), prefer using that method in the test to update the aggregate before calling UpdateAsync, instead of relying on reflection that can break under trimming/AOT and doesn't reflect real usage.

Copilot · 2026-04-07T13:51:20Z

        writeContext.Entry(originalCategory).State = EntityState.Detached;

        var repository = new CategoryRepository(readContext, writeContext);
-        originalCategory.Title = "Atualizado";
+        typeof(Category).GetProperty("Title")?.SetValue(originalCategory, "Atualizado");

        await repository.UpdateAsync(originalCategory);


This test updates Category.Title via reflection to bypass the domain encapsulation. Since Category now provides an Update(title, description) method (with validation), prefer using it so the test stays aligned with the aggregate behavior and avoids brittle reflection.

- Remove member initializer from Result<T>.Value (set in constructor) - Initialize Errors list in constructor with null-coalescing - Make Result class static to prevent instantiation - Add null checks before accessing title.Length in Category

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

- AuthController: map Email in UpdateProfileCommand - TransactionHandlerTests: fix parameter order (categoryId, userId) - UpdateProfileHandler: use request.Email instead of user.Email - TransactionMapper: add CategoryId mapping - CategoryDto.cs: separate TransactionDto into own file - Remove duplicate ETransactionType from Contracts - Domain/TransactionDto: remove unused using - CategoryRepositoryTests: use Update method instead of reflection - TransactionRepositoryTests: fix Create parameter order - UserRepositoryTests: use UpdateProfile instead of reflection

…onsz/Finance-API into refactor/clean-architecture

Fix CS0101 error by replacing accidental CategoryDto with proper TransactionDto class

sonarqubecloud · 2026-04-07T14:46:06Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.4% Duplication on New Code

See analysis details on SonarQube Cloud

alysonsz added 9 commits April 7, 2026 09:59

feat(contracts): centralize DTOs and Enums

ed4a4e7

- Add CategoryDto, TransactionDto, UserDto to Contracts.DTOs - Add ETransactionType enum to Contracts.Enums - Remove DTOs from Domain.Models.DTOs namespace

feat(infrastructure): update repository implementations

7bfb74a

- Update CategoryRepository to use Application.Interfaces - Update TransactionRepository to use Application.Interfaces - Update UserRepository to use Application.Interfaces

refactor: remove obsolete service layer

78b66aa

- Delete CategoryService, TransactionService, UserService - Delete ICategoryService, ITransactionService, IUserService interfaces

docs: update README with new architecture structure

de68aa5

- Update project structure to reflect Clean Architecture - Add Finance.Domain.Tests and separate test projects

ci: enable workflow on refactor/clean-architecture branch

7b3d7c3

Copilot AI review requested due to automatic review settings April 7, 2026 13:43

Copilot started reviewing on behalf of alysonsz April 7, 2026 13:44 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

alysonsz and others added 6 commits April 7, 2026 10:53

apply suggestion from @Copilot

6f2a497

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Merge branch 'refactor/clean-architecture' of https://github.com/alys…

57ad6e6

…onsz/Finance-API into refactor/clean-architecture

fix: correct TransactionDto class definition

dc74a72

Fix CS0101 error by replacing accidental CategoryDto with proper TransactionDto class

test: add Email to UpdateProfileCommand test

8408617

alysonsz merged commit 787603d into main Apr 7, 2026
3 checks passed

alysonsz deleted the refactor/clean-architecture branch April 7, 2026 14:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor/clean architecture#3

refactor/clean architecture#3
alysonsz merged 15 commits into
mainfrom
refactor/clean-architecture

alysonsz commented Apr 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

sonarqubecloud Bot commented Apr 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	var updateResult = user.UpdateProfile(request.Name, user.Email);
	var updateResult = user.UpdateProfile(request.Name, request.Email);

Conversation

alysonsz commented Apr 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Apr 7, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants