Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create payment #104

Merged
merged 8 commits into from
Jun 19, 2024
Merged

Create payment #104

merged 8 commits into from
Jun 19, 2024

Conversation

italopessoa
Copy link
Member

No description provided.

Copy link

coderabbitai bot commented Jun 16, 2024

Walkthrough

The update introduces a new payment handling feature to the FIAP.TechChallenge.ByteMeBurger system. It includes the addition and integration of payment entities, view models, a payment controller, a MercadoPago service, and an SQL table for payments. Additionally, there are changes to the orders controller to modify order statuses and enhanced exception handling across the API.

Changes

File(s) Change Summary
src/.../PaymentViewModel.cs, src/.../PaymentId.cs, src/.../PaymentStatus.cs Added new files defining payment-related models, enums, and value objects.
src/.../Payment.cs Added Payment class with properties, constructors, and methods.
src/.../PaymentController.cs Added PaymentController for handling payment-related operations.
src/.../IPaymentService.cs, src/.../IPaymentRepository.cs Introduced IPaymentService and IPaymentRepository interfaces with methods for creating and managing payments.
src/.../MercadoPagoService.cs Implemented MercadoPagoService class, integrating payment creation with MercadoPago API.
src/.../OrdersController.cs Modified UpdateStatus method to change order status using PATCH instead of PUT.
src/.../DomainExceptionHandler.cs, src/.../GlobalExceptionHandler.cs Introduced exception handler classes for domain-specific and global exception handling.
src/.../Program.cs Updated Program.cs to integrate new exception handling services.
src/.../Constants.cs Added SQL queries as constants for order and payment operations.
src/.../OrderItemDto.cs, src/.../OrderListDto.cs, src/.../PaymentDAO.cs Added or modified DTOs to include necessary properties for order items and payments.
src/.../OrderRepositoryDapper.cs, src/.../PaymentRepositoryDapper.cs Updated repository classes to use new SQL queries and handle payments.
tests/.../PaymentRepositoryDapperTest.cs Added unit tests for PaymentRepositoryDapper.
database/init.sql Added new SQL table for payments.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant PaymentController
    participant PaymentService
    participant PaymentRepository
    participant MercadoPagoService
    
    Client->>PaymentController: Create payment request with orderId
    PaymentController->>PaymentService: CreateOrderPaymentAsync(orderId)
    PaymentService->>PaymentRepository: SaveAsync(payment)
    PaymentService->>MercadoPagoService: CreatePaymentAsync(order)
    MercadoPagoService->>PaymentRepository: SaveAsync(mpResponse)
    PaymentService->>PaymentController: return PaymentViewModel
    PaymentController->>Client: return PaymentViewModel
Loading

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2932d19 and 20a86ed.

Files selected for processing (28)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/Configuration/MySqlSettings.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/DomainEventsHandler.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/Model/PaymentViewModel.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/ServiceCollectionExtensions.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Application/FIAP.TechChallenge.ByteMeBurger.Application.csproj (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Application/ServiceCollectionsExtensions.cs (2 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Application/Services/PaymentService.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Application/UseCases/Payment/CreatePaymentUseCase.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Application/UseCases/Payment/ICreatePaymentUseCase.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Base/Entity.cs (2 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Entities/Order.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Entities/Payment.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Interfaces/IPaymentGateway.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Interfaces/IPaymentRepository.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Interfaces/IPaymentService.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/ValueObjects/PaymentId.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/ValueObjects/PaymentStatus.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/Configuration/MercadoPagoOptions.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway.csproj (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/MercadoPagoService.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/Security/MercadoPagoHmacSignatureValidator.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/ServiceCollectionsExtensions.cs (2 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Persistence/Repository/PaymentRepositoryDapper.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Persistence/ServiceExtensions.cs (3 hunks)
  • tests/FIAP.TechChallenge.ByteMeBurger.Api.Test/Controllers/PaymentControllerTest.cs (1 hunks)
  • tests/FIAP.TechChallenge.ByteMeBurger.Api.Test/FIAP.TechChallenge.ByteMeBurger.Api.Test.csproj (1 hunks)
  • tests/FIAP.TechChallenge.ByteMeBurger.Application.Test/Services/PaymentServiceTests.cs (1 hunks)
  • tests/FIAP.TechChallenge.ByteMeBurger.Application.Test/UseCases/Payments/CreatePaymentUseCaseTest.cs (1 hunks)
Files not reviewed due to errors (5)
  • src/FIAP.TechChallenge.ByteMeBurger.Persistence/Repository/PaymentRepositoryDapper.cs (no review received)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Entities/Payment.cs (no review received)
  • src/FIAP.TechChallenge.ByteMeBurger.Application/Services/PaymentService.cs (no review received)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/ServiceCollectionExtensions.cs (no review received)
  • src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/ServiceCollectionsExtensions.cs (no review received)
Files skipped from review due to trivial changes (6)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/Model/PaymentViewModel.cs
  • src/FIAP.TechChallenge.ByteMeBurger.Application/FIAP.TechChallenge.ByteMeBurger.Application.csproj
  • src/FIAP.TechChallenge.ByteMeBurger.Application/UseCases/Payment/ICreatePaymentUseCase.cs
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/ValueObjects/PaymentId.cs
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/ValueObjects/PaymentStatus.cs
  • tests/FIAP.TechChallenge.ByteMeBurger.Api.Test/FIAP.TechChallenge.ByteMeBurger.Api.Test.csproj
Additional context used
Path-based instructions (21)
src/FIAP.TechChallenge.ByteMeBurger.Domain/Interfaces/IPaymentRepository.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Domain/Interfaces/IPaymentGateway.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Domain/Interfaces/IPaymentService.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Domain/Base/Entity.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Persistence/Repository/PaymentRepositoryDapper.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/Configuration/MercadoPagoOptions.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Api/Configuration/MySqlSettings.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Domain/Entities/Payment.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Application/Services/PaymentService.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Api/ServiceCollectionExtensions.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/ServiceCollectionsExtensions.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Application/UseCases/Payment/CreatePaymentUseCase.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Persistence/ServiceExtensions.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

tests/FIAP.TechChallenge.ByteMeBurger.Application.Test/Services/PaymentServiceTests.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

tests/FIAP.TechChallenge.ByteMeBurger.Api.Test/Controllers/PaymentControllerTest.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Application/ServiceCollectionsExtensions.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Api/DomainEventsHandler.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

tests/FIAP.TechChallenge.ByteMeBurger.Application.Test/UseCases/Payments/CreatePaymentUseCaseTest.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/Security/MercadoPagoHmacSignatureValidator.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Domain/Entities/Order.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/MercadoPagoService.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

GitHub Check: build
src/FIAP.TechChallenge.ByteMeBurger.Domain/Base/Entity.cs

[warning] 5-5:
Non-nullable property 'Id' must contain a non-null value when exiting constructor. Consider declaring the property as nullable.


[warning] 29-29:
Dereference of a possibly null reference.

src/FIAP.TechChallenge.ByteMeBurger.Domain/Entities/Payment.cs

[warning] 22-22:
Non-nullable property 'SystemId' must contain a non-null value when exiting constructor. Consider declaring the property as nullable.


[warning] 22-22:
Non-nullable property 'QrCode' must contain a non-null value when exiting constructor. Consider declaring the property as nullable.


[warning] 26-26:
Non-nullable property 'SystemId' must contain a non-null value when exiting constructor. Consider declaring the property as nullable.


[warning] 26-26:
Non-nullable property 'QrCode' must contain a non-null value when exiting constructor. Consider declaring the property as nullable.


[warning] 34-34:
Non-nullable property 'SystemId' must contain a non-null value when exiting constructor. Consider declaring the property as nullable.

tests/FIAP.TechChallenge.ByteMeBurger.Api.Test/Controllers/PaymentControllerTest.cs

[failure] 20-20:
The type or namespace name 'PaymentController' could not be found (are you missing a using directive or an assembly reference?)


[failure] 24-24:
The type or namespace name 'PaymentController' could not be found (are you missing a using directive or an assembly reference?)


[failure] 20-20:
The type or namespace name 'PaymentController' could not be found (are you missing a using directive or an assembly reference?)


[failure] 24-24:
The type or namespace name 'PaymentController' could not be found (are you missing a using directive or an assembly reference?)

src/FIAP.TechChallenge.ByteMeBurger.Domain/Entities/Order.cs

[warning] 32-32:
Non-nullable property 'TrackingCode' must contain a non-null value when exiting constructor. Consider declaring the property as nullable.


[warning] 42-42:
Non-nullable property 'TrackingCode' must contain a non-null value when exiting constructor. Consider declaring the property as nullable.


[warning] 48-48:
Non-nullable property 'TrackingCode' must contain a non-null value when exiting constructor. Consider declaring the property as nullable.

Additional comments not posted (20)
src/FIAP.TechChallenge.ByteMeBurger.Domain/Interfaces/IPaymentRepository.cs (1)

13-13: Ensure proper error handling in SaveAsync method.

While the method signature is straightforward, it's critical to ensure that the implementation handles potential exceptions gracefully, especially given the asynchronous nature of database operations. Please verify that the implementation of this method includes adequate error handling mechanisms.

src/FIAP.TechChallenge.ByteMeBurger.Domain/Interfaces/IPaymentGateway.cs (1)

13-13: Ensure CreatePaymentAsync adheres to the interface contract.

The method signature is well-defined, aligning with the expected responsibilities of a payment gateway. Ensure that the implementation adheres to the interface contract and handles exceptions and edge cases appropriately.

src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway.csproj (1)

10-18: Ensure package versions are compatible with the project.

The inclusion of mercadopago-sdk and other Microsoft packages, along with a project reference to FIAP.TechChallenge.ByteMeBurger.Domain, supports modular architecture and should be validated for compatibility with the rest of the project.

src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/Configuration/MercadoPagoOptions.cs (1)

12-27: Properly validated new configuration properties.

The addition of WebhookSecret, AccessToken, and NotificationUrl with [Required] and [MinLength(1)] attributes ensures that these essential configuration properties are correctly set up, enhancing the robustness and reliability of the payment gateway configuration.

src/FIAP.TechChallenge.ByteMeBurger.Api/Configuration/MySqlSettings.cs (1)

12-13: Marking MySqlSettings as obsolete is a clear directive for migration.

The [Obsolete] attribute effectively signals that this class will be phased out in favor of using the ConnectionStrings section in appsettings.json. Ensure that the migration path is well-documented to assist developers in transitioning smoothly.

src/FIAP.TechChallenge.ByteMeBurger.Persistence/ServiceExtensions.cs (1)

Line range hint 18-36: ### Review of ServiceExtensions.ConfigurePersistenceApp
The method ConfigurePersistenceApp is correctly setting up dependencies for the persistence layer. However, the method could benefit from:

  1. Connection String Safety: Ensure that the connection string does not expose sensitive information and is retrieved securely.
  2. Service Registration: Good use of dependency injection. Consider adding comments for clarity.
tests/FIAP.TechChallenge.ByteMeBurger.Application.Test/Services/PaymentServiceTests.cs (1)

14-44: ### Review of PaymentServiceTests
The unit tests for PaymentService are comprehensive and well-structured. Here are some suggestions:

  1. Mock Verification: Ensure that all necessary methods of the mocks are verified to check if they are being called as expected.
  2. Test Coverage: Consider adding negative test cases and edge cases to improve the robustness of the tests.
src/FIAP.TechChallenge.ByteMeBurger.Application/ServiceCollectionsExtensions.cs (4)

13-13: Ensure that the exclusion from code coverage is justified.

Consider verifying if the exclusion from code coverage for ServiceCollectionsExtensions is justified, as this class is central to dependency injection and might need to be tested, especially when new services are added.


21-21: Good addition of payment use cases to the service collection.

This enhances modularity by clearly separating the concerns of different application functionalities.


49-52: Well-structured method for adding payment use cases.

The method AddPaymentUseCases follows the established pattern and correctly encapsulates the registration of payment-related use cases.


60-60: Good integration of the PaymentService into the service collection.

This ensures that PaymentService can be injected wherever needed, adhering to the principles of dependency inversion.

tests/FIAP.TechChallenge.ByteMeBurger.Application.Test/UseCases/Payments/CreatePaymentUseCaseTest.cs (4)

14-14: Excellent use of attribute to specify the test subject.

This helps in maintaining clarity and focus during testing, making it easier to understand which class or method is being tested.


28-55: Comprehensive test for valid order payment creation.

This test method thoroughly checks the creation of a payment from a valid order, ensuring that all parts of the process are functioning as expected.


57-73: Robust testing for order not found scenario.

The test correctly anticipates and handles a scenario where an order does not exist, which is crucial for robust error handling in the payment creation process.


75-93: Effective testing for duplicate payment prevention.

This test ensures that the system correctly prevents creating a payment for an order that already has one, which is vital for maintaining data integrity.

src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/Security/MercadoPagoHmacSignatureValidator.cs (1)

94-95: Updated validation logic to include both "data.id" and "data_id".

This change enhances the robustness of the validation by accommodating different parameter naming conventions that might be used by clients.

src/FIAP.TechChallenge.ByteMeBurger.Domain/Entities/Order.cs (1)

30-31: Added a new PaymentId property to the Order class to link payments with orders.

src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/MercadoPagoService.cs (3)

20-22: Class MercadoPagoService correctly implements the IPaymentGateway interface.


25-30: Ensure the constructor correctly handles configuration settings.


32-127: The CreatePaymentAsync method implements the payment creation logic using MercadoPago.

However, consider refactoring to improve readability and maintainability:

  • Extract methods like MapPaymentCreateRequest and MapPaymentPayerRequest outside of CreatePaymentAsync to enhance readability and reusability.
  • Simplify the method by breaking down complex logic into smaller, more manageable functions.

@@ -27,7 +27,9 @@

public decimal Total => _orderItems.Sum(o => o.UnitPrice * o.Quantity);

public PaymentId? PaymentId { get; set; }

public Order()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure non-nullable properties are correctly initialized in all constructors.

- public OrderTrackingCode TrackingCode { get; private set; }
+ public OrderTrackingCode? TrackingCode { get; private set; }

Consider modifying the property TrackingCode to be nullable or ensure it is initialized in all constructors to prevent runtime null reference issues.

Also applies to: 42-42, 48-48

Tools
GitHub Check: build

[warning] 32-32:
Non-nullable property 'TrackingCode' must contain a non-null value when exiting constructor. Consider declaring the property as nullable.


public interface IPaymentService
{
Task<Payment> CreateOrderPaymentAsync(Guid order);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarify parameter naming in CreateOrderPaymentAsync.

- Task<Payment> CreateOrderPaymentAsync(Guid order);
+ Task<Payment> CreateOrderPaymentAsync(Guid orderId);

The parameter name order in CreateOrderPaymentAsync might be misleading as it suggests an Order object rather than a Guid. Consider renaming it to orderId to improve clarity and prevent potential misunderstandings in the usage of this API.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Task<Payment> CreateOrderPaymentAsync(Guid order);
Task<Payment> CreateOrderPaymentAsync(Guid orderId);

public TId Id { get; protected set; }

public TId Id { get; set; }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address non-nullable property initialization issue.

- public TId Id { get; set; }
+ public TId? Id { get; set; }

The property Id is non-nullable and must be initialized before exiting the constructor. Consider declaring the property as nullable (TId?) if null values are acceptable, or ensure it is always initialized within the constructor.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public TId Id { get; set; }
public TId? Id { get; set; }

@@ -26,6 +26,6 @@

public override int GetHashCode()
{
return Id.GetHashCode();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review null reference in GetHashCode.

- return Id.GetHashCode();
+ return Id?.GetHashCode() ?? 0;

There's a potential dereference of a possibly null reference in the GetHashCode method. Modifying the code to handle possible null values can prevent runtime exceptions.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return Id.GetHashCode();
return Id?.GetHashCode() ?? 0;
Tools
GitHub Check: build

[warning] 29-29:
Dereference of a possibly null reference.

Comment on lines +12 to +34
public class CreatePaymentUseCase : ICreatePaymentUseCase
{
private readonly IPaymentGateway _paymentGateway;
private readonly IOrderRepository _orderRepository;

public CreatePaymentUseCase(IPaymentGateway paymentGateway, IOrderRepository orderRepository)
{
_paymentGateway = paymentGateway;
_orderRepository = orderRepository;
}

public async Task<Domain.Entities.Payment> Execute(Guid orderId)
{
var order = await _orderRepository.GetAsync(orderId);

if (order is null)
throw new EntityNotFoundException("Order not found.");

if (order.PaymentId is not null)
throw new DomainException("There's already a Payment for the order.");

return await _paymentGateway.CreatePaymentAsync(order);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of CreatePaymentUseCase class

The CreatePaymentUseCase class is well-structured and follows clean architecture principles. However, there are a couple of potential improvements:

  1. Exception Handling: While the current implementation throws exceptions for not found orders and existing payments, consider adding more specific error messages or handling potential exceptions from the _paymentGateway.CreatePaymentAsync method.
  2. Asynchronous Method Naming: By convention, asynchronous methods should have "Async" suffix to indicate their nature clearly.
- public async Task<Domain.Entities.Payment> Execute(Guid orderId)
+ public async Task<Domain.Entities.Payment> ExecuteAsync(Guid orderId)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public class CreatePaymentUseCase : ICreatePaymentUseCase
{
private readonly IPaymentGateway _paymentGateway;
private readonly IOrderRepository _orderRepository;
public CreatePaymentUseCase(IPaymentGateway paymentGateway, IOrderRepository orderRepository)
{
_paymentGateway = paymentGateway;
_orderRepository = orderRepository;
}
public async Task<Domain.Entities.Payment> Execute(Guid orderId)
{
var order = await _orderRepository.GetAsync(orderId);
if (order is null)
throw new EntityNotFoundException("Order not found.");
if (order.PaymentId is not null)
throw new DomainException("There's already a Payment for the order.");
return await _paymentGateway.CreatePaymentAsync(order);
}
public class CreatePaymentUseCase : ICreatePaymentUseCase
{
private readonly IPaymentGateway _paymentGateway;
private readonly IOrderRepository _orderRepository;
public CreatePaymentUseCase(IPaymentGateway paymentGateway, IOrderRepository orderRepository)
{
_paymentGateway = paymentGateway;
_orderRepository = orderRepository;
}
public async Task<Domain.Entities.Payment> ExecuteAsync(Guid orderId)
{
var order = await _orderRepository.GetAsync(orderId);
if (order is null)
throw new EntityNotFoundException("Order not found.");
if (order.PaymentId is not null)
throw new DomainException("There's already a Payment for the order.");
return await _paymentGateway.CreatePaymentAsync(order);
}

Comment on lines 20 to 54
[TestSubject(typeof(PaymentController))]
public class PaymentControllerTest
{
private readonly Mock<IPaymentService> _serviceMock;
private readonly PaymentController _target;

public PaymentControllerTest()
{
_serviceMock = new Mock<IPaymentService>();
_target = new PaymentController(_serviceMock.Object);
}

[Fact]
public async void Create_Success()
{
// Arrange
var paymentId = new PaymentId("123", 1);
var payment = new Payment(paymentId, Guid.NewGuid(), "qrcode");
_serviceMock.Setup(p => p.CreateOrderPaymentAsync(It.IsAny<Guid>()))
.ReturnsAsync(payment);

// Act
var response = await _target.Create(Guid.NewGuid(), CancellationToken.None);

// Assert
using (new AssertionScope())
{
response.Result.Should().BeOfType<OkObjectResult>();
var paymentViewModel = response.Result.As<OkObjectResult>().Value.As<PaymentViewModel>();

paymentViewModel.PaymentId.Should().Be(payment.Id.ExternalReference);
paymentViewModel.VendorId.Should().Be(payment.Id.SystemId.ToString());
paymentViewModel.QrCode.Should().Be(payment.QrCode);
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of PaymentControllerTest

The tests for PaymentController cover the basic functionality. However, the build failures indicate a missing reference or using directive for PaymentController. Ensure the controller is correctly referenced in the project.

+ using FIAP.TechChallenge.ByteMeBurger.Api.Controllers;
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[TestSubject(typeof(PaymentController))]
public class PaymentControllerTest
{
private readonly Mock<IPaymentService> _serviceMock;
private readonly PaymentController _target;
public PaymentControllerTest()
{
_serviceMock = new Mock<IPaymentService>();
_target = new PaymentController(_serviceMock.Object);
}
[Fact]
public async void Create_Success()
{
// Arrange
var paymentId = new PaymentId("123", 1);
var payment = new Payment(paymentId, Guid.NewGuid(), "qrcode");
_serviceMock.Setup(p => p.CreateOrderPaymentAsync(It.IsAny<Guid>()))
.ReturnsAsync(payment);
// Act
var response = await _target.Create(Guid.NewGuid(), CancellationToken.None);
// Assert
using (new AssertionScope())
{
response.Result.Should().BeOfType<OkObjectResult>();
var paymentViewModel = response.Result.As<OkObjectResult>().Value.As<PaymentViewModel>();
paymentViewModel.PaymentId.Should().Be(payment.Id.ExternalReference);
paymentViewModel.VendorId.Should().Be(payment.Id.SystemId.ToString());
paymentViewModel.QrCode.Should().Be(payment.QrCode);
}
}
using FIAP.TechChallenge.ByteMeBurger.Api.Controllers;
[TestSubject(typeof(PaymentController))]
public class PaymentControllerTest
{
private readonly Mock<IPaymentService> _serviceMock;
private readonly PaymentController _target;
public PaymentControllerTest()
{
_serviceMock = new Mock<IPaymentService>();
_target = new PaymentController(_serviceMock.Object);
}
[Fact]
public async void Create_Success()
{
// Arrange
var paymentId = new PaymentId("123", 1);
var payment = new Payment(paymentId, Guid.NewGuid(), "qrcode");
_serviceMock.Setup(p => p.CreateOrderPaymentAsync(It.IsAny<Guid>()))
.ReturnsAsync(payment);
// Act
var response = await _target.Create(Guid.NewGuid(), CancellationToken.None);
// Assert
using (new AssertionScope())
{
response.Result.Should().BeOfType<OkObjectResult>();
var paymentViewModel = response.Result.As<OkObjectResult>().Value.As<PaymentViewModel>();
paymentViewModel.PaymentId.Should().Be(payment.Id.ExternalReference);
paymentViewModel.VendorId.Should().Be(payment.Id.SystemId.ToString());
paymentViewModel.QrCode.Should().Be(payment.QrCode);
}
}
Tools
GitHub Check: build

[failure] 20-20:
The type or namespace name 'PaymentController' could not be found (are you missing a using directive or an assembly reference?)


[failure] 24-24:
The type or namespace name 'PaymentController' could not be found (are you missing a using directive or an assembly reference?)


[failure] 20-20:
The type or namespace name 'PaymentController' could not be found (are you missing a using directive or an assembly reference?)


[failure] 24-24:
The type or namespace name 'PaymentController' could not be found (are you missing a using directive or an assembly reference?)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Outside diff range and nitpick comments (1)
database/init.sql (1)

54-54: Consider using only Id as the primary key unless there's a specific requirement for a composite key with OrderId.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 20a86ed and 700d3ab.

Files selected for processing (6)
  • database/init.sql (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/Model/PaymentViewModel.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Entities/Payment.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/ValueObjects/PaymentId.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/MercadoPagoService.cs (1 hunks)
  • tests/FIAP.TechChallenge.ByteMeBurger.Api.Test/Controllers/PaymentControllerTest.cs (1 hunks)
Files skipped from review as they are similar to previous changes (4)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/Model/PaymentViewModel.cs
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Entities/Payment.cs
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/ValueObjects/PaymentId.cs
  • tests/FIAP.TechChallenge.ByteMeBurger.Api.Test/Controllers/PaymentControllerTest.cs
Additional context used
Path-based instructions (1)
src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/MercadoPagoService.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

Additional comments not posted (3)
src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/MercadoPagoService.cs (3)

25-30: The constructor properly validates the WebhookSecret before use, ensuring that essential configuration is not missed.


34-41: Using an idempotency key based on the order ID is a robust choice to prevent duplicate transactions.


115-126: The async API call and the robust handling of the response with a fallback payment status demonstrate good error handling and resilience.

Comment on lines 43 to 92
PaymentCreateRequest MapPaymentCreateRequest(PaymentPayerRequest paymentPayerRequest2,
PaymentAdditionalInfoRequest paymentAdditionalInfoRequest)
{
return new PaymentCreateRequest
{
Description = $"Payment for Order {order.TrackingCode.Value}",
ExternalReference = order.TrackingCode.Value,
Installments = 1,
NotificationUrl = _mercadoPagoOptions.NotificationUrl,
Payer = paymentPayerRequest2,
PaymentMethodId = "pix",
StatementDescriptor = "tech challenge restaurant",
TransactionAmount = (decimal?)0.01,
AdditionalInfo = paymentAdditionalInfoRequest,
DateOfExpiration = DateTime.UtcNow.AddMinutes(5)
};
}

PaymentPayerRequest MapPaymentPayerRequest()
=> new()
{
Type = "individual",
EntityType = "customer",
Email = order.Customer.Email,
FirstName = order.Customer.Name,
LastName = "User",
Identification = new IdentificationRequest
{
Type = "CPF",
Number = order.Customer.Cpf
},
};

IEnumerable<PaymentItemRequest> MapPaymentItemRequests() => order.OrderItems.Select(item =>
new PaymentItemRequest
{
Id = item.Id.ToString(),
Title = item.ProductName,
Description = item.ProductName,
CategoryId = "food",
Quantity = item.Quantity,
UnitPrice = item.UnitPrice,
Warranty = false,
CategoryDescriptor = new PaymentCategoryDescriptorRequest
{
Passenger = { },
Route = { }
}
}
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider refactoring the local mapping functions to private methods of the class to improve readability and maintainability of the CreatePaymentAsync method.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 700d3ab and eab7ff5.

Files selected for processing (4)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/Controllers/PaymentController.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Interfaces/IPaymentService.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/MercadoPagoService.cs (1 hunks)
  • tests/FIAP.TechChallenge.ByteMeBurger.Application.Test/UseCases/Payments/CreatePaymentUseCaseTest.cs (1 hunks)
Files not reviewed due to errors (2)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/Controllers/PaymentController.cs (no review received)
  • src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/MercadoPagoService.cs (no review received)
Files skipped from review as they are similar to previous changes (2)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Interfaces/IPaymentService.cs
  • tests/FIAP.TechChallenge.ByteMeBurger.Application.Test/UseCases/Payments/CreatePaymentUseCaseTest.cs
Additional context used
Path-based instructions (2)
src/FIAP.TechChallenge.ByteMeBurger.Api/Controllers/PaymentController.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.MercadoPago.Gateway/MercadoPagoService.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

- create Constants class for scripts
- add unit tests
- add error handlers
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between eab7ff5 and 6bb6152.

Files selected for processing (15)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/Controllers/OrdersController.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/DomainExceptionHandler.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/GlobalExceptionHandler.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/Program.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Entities/Payment.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Interfaces/IPaymentRepository.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/ValueObjects/PaymentStatus.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Persistence/Constants.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Persistence/Dto/OrderItemDto.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Persistence/Dto/OrderListDto.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Persistence/Dto/PaymentDAO.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Persistence/Repository/OrderRepositoryDapper.cs (4 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Persistence/Repository/PaymentRepositoryDapper.cs (1 hunks)
  • tests/FIAP.TechChallenge.ByteMeBurger.Api.Test/FIAP.TechChallenge.ByteMeBurger.Api.Test.csproj (2 hunks)
  • tests/FIAP.TechChallenge.ByteMeBurger.Api.Test/Repository/PaymentRepositoryDapperTest.cs (1 hunks)
Files skipped from review due to trivial changes (2)
  • src/FIAP.TechChallenge.ByteMeBurger.Persistence/Dto/OrderItemDto.cs
  • src/FIAP.TechChallenge.ByteMeBurger.Persistence/Dto/OrderListDto.cs
Files skipped from review as they are similar to previous changes (5)
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Entities/Payment.cs
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/Interfaces/IPaymentRepository.cs
  • src/FIAP.TechChallenge.ByteMeBurger.Domain/ValueObjects/PaymentStatus.cs
  • src/FIAP.TechChallenge.ByteMeBurger.Persistence/Repository/PaymentRepositoryDapper.cs
  • tests/FIAP.TechChallenge.ByteMeBurger.Api.Test/FIAP.TechChallenge.ByteMeBurger.Api.Test.csproj
Additional context used
Path-based instructions (8)
src/FIAP.TechChallenge.ByteMeBurger.Persistence/Dto/PaymentDAO.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Api/GlobalExceptionHandler.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Api/DomainExceptionHandler.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

tests/FIAP.TechChallenge.ByteMeBurger.Api.Test/Repository/PaymentRepositoryDapperTest.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Persistence/Constants.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Api/Program.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Api/Controllers/OrdersController.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

src/FIAP.TechChallenge.ByteMeBurger.Persistence/Repository/OrderRepositoryDapper.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.

Additional comments not posted (12)
src/FIAP.TechChallenge.ByteMeBurger.Persistence/Constants.cs (5)

11-30: The SQL query for fetching order details by ID is well-structured and uses parameterization to prevent SQL injection.


32-50: The SQL query for fetching all order details is correctly implemented with necessary joins and parameterization for security.


52-53: This update query for order status is secure and efficient, using parameterization to safeguard against SQL injection.


55-56: The insertion query for new orders is properly parameterized and follows best practices for database interactions.
[APROVED]


62-64: The SQL query for inserting payment records is secure and adheres to best practices with its use of parameterization.

src/FIAP.TechChallenge.ByteMeBurger.Api/Program.cs (3)

90-92: Adding DomainExceptionHandler, GlobalExceptionHandler, and ProblemDetails enhances error handling and aligns with RESTful service best practices.

Also applies to: 97-97


83-83: The configuration to serialize enums as strings in JSON responses is a good practice, enhancing readability and client-side usability.


83-92: The service configuration and middleware setup, including CORS, Swagger, and health checks, are appropriately implemented to support the API's functionality and security.

Also applies to: 97-97

src/FIAP.TechChallenge.ByteMeBurger.Api/Controllers/OrdersController.cs (1)

107-113: Changing the HTTP method from PUT to PATCH for the UpdateStatus method is appropriate for partial updates, aligning with RESTful best practices.

src/FIAP.TechChallenge.ByteMeBurger.Persistence/Repository/OrderRepositoryDapper.cs (3)

Line range hint 31-40: The CreateAsync method correctly handles database transactions, uses parameterized queries to prevent SQL injection, and logs actions appropriately.


64-88: The GetAllAsync method is efficiently implemented with appropriate use of Dapper's multi-mapping capabilities to handle complex queries and multiple result sets.


140-140: The UpdateOrderStatusAsync method effectively updates the order status and logs the operation, using best practices for database interaction.

Comment on lines +9 to +40
public class PaymentDAO
{
public string Id { get; init; }
public Guid OrderId { get; init; }
public int Status { get; init; }
public int PaymentType { get; init; }
public decimal Amount { get; init; }
public DateTime Created { get; init; }
public DateTime? Updated { get; init; }

public PaymentDAO()
{
}

public PaymentDAO(
string id,
Guid orderId,
int status,
int paymentType,
decimal amount,
DateTime created,
DateTime? updated)
{
Id = id;
OrderId = orderId;
Status = status;
PaymentType = paymentType;
Amount = amount;
Created = created;
Updated = updated;
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class definition for PaymentDAO is clear and straightforward, encapsulating all necessary properties for a payment data transfer object. The use of init for properties ensures immutability after construction, which is a good practice. However, consider adding XML documentation comments above the class and its constructors to improve code maintainability and provide better context for other developers.

Comment on lines +14 to +35
public class GlobalExceptionHandler(ILogger<GlobalExceptionHandler> logger) : IExceptionHandler
{
public async ValueTask<bool> TryHandleAsync(HttpContext httpContext, Exception exception,
CancellationToken cancellationToken)
{
logger.LogError(
exception, "Exception occurred: {Message}", exception.Message);

var problemDetails = new ProblemDetails
{
Status = StatusCodes.Status500InternalServerError,
Title = "Server error",
Detail = "Something went wrong",
};

httpContext.Response.StatusCode = problemDetails.Status.Value;

await httpContext.Response
.WriteAsJsonAsync(problemDetails, cancellationToken);

return true;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation of GlobalExceptionHandler adheres to best practices by centralizing exception handling and logging. This approach enhances maintainability and ensures that all unhandled exceptions are processed in a consistent manner. However, consider enriching the ProblemDetails with more specific error information if available, and ensure that the HTTP response includes all necessary headers.

Comment on lines +15 to +37
public class DomainExceptionHandler(ILogger<GlobalExceptionHandler> logger) : IExceptionHandler
{
public async ValueTask<bool> TryHandleAsync(HttpContext httpContext, Exception exception,
CancellationToken cancellationToken)
{
logger.LogWarning(
exception, "Exception occurred: {Message}", exception.Message);

if (exception is not DomainException)
return false;

var problemDetails = new ProblemDetails
{
Status = exception is EntityNotFoundException ? StatusCodes.Status404NotFound : StatusCodes.Status400BadRequest,
Title = "The request could not be completed.",
Detail = exception.Message,
};
httpContext.Response.StatusCode = problemDetails.Status.Value;
await httpContext.Response
.WriteAsJsonAsync(problemDetails, cancellationToken);

return true;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DomainExceptionHandler class is well-implemented, focusing on domain-specific exceptions. The use of conditional checks to provide different HTTP statuses based on the type of exception is a good practice. However, consider adding more specific error logging for different types of domain exceptions to aid in debugging and operational monitoring.

Comment on lines +22 to +56
[TestSubject(typeof(PaymentRepositoryDapper))]
public class PaymentRepositoryDapperTest
{
private readonly Mock<IDbConnection> _mockConnection;
private readonly PaymentRepositoryDapper _target;

public PaymentRepositoryDapperTest()
{
_mockConnection = new Mock<IDbConnection>();
_target = new PaymentRepositoryDapper(_mockConnection.Object, Mock.Of<ILogger<PaymentRepositoryDapper>>());
}

[Fact]
public async Task Create_Success()
{
// Arrange
var expectedPayment = new Fixture().Create<Payment>();

_mockConnection.Setup(c => c.BeginTransaction()).Returns(Mock.Of<IDbTransaction>());

_mockConnection.SetupDapperAsync(c =>
c.ExecuteAsync(Constants.InsertPaymentQuery,
null, null, null, null))
.ReturnsAsync(1);

// Act
var result = await _target.SaveAsync(expectedPayment);

// Assert
using (new AssertionScope())
{
result.Should().NotBeNull();
result.Should().Be(expectedPayment);
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test method Create_Success in PaymentRepositoryDapperTest is well-structured and covers the basic functionality of the SaveAsync method. Using FluentAssertions and AssertionScope provides clear and expressive assertions. However, the test lacks a setup for the parameters passed to ExecuteAsync, which might lead to false positives during testing. Consider mocking the parameters to ensure they are passed correctly to the method.

- null, null, null, null
+ new { expectedPayment.Id, expectedPayment.OrderId, ... }, It.IsAny<IDbTransaction>(), null, null

Committable suggestion was skipped due to low confidence.

@italopessoa italopessoa merged commit feb8c0c into main Jun 19, 2024
2 checks passed
@italopessoa italopessoa deleted the create_payment branch June 24, 2024 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant