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

feat: add endpoint integration to create products #30

Merged
merged 1 commit into from
May 2, 2024

Conversation

italopessoa
Copy link
Member

@italopessoa italopessoa linked an issue May 1, 2024 that may be closed by this pull request
Copy link

coderabbitai bot commented May 1, 2024

Walkthrough

Recent updates in the project primarily focus on refining the database schema, enhancing exception handling, and modifying service registrations. There's an addition of a new testing suite for the ProductRepositoryDapper class, ensuring robustness in product management functionalities. The changes collectively aim to improve data handling, resource cleanup, and testing efficacy across the system.

Changes

File Path Change Summary
.docker/setup.sql Renamed table Product to Products.
src/.../Controllers/ProductController.cs Enhanced exception handling in Delete method by explicitly catching exceptions.
src/.../ServiceCollectionExtensions.cs Commented out registration of IProductRepository using InMemoryProductRepository.
src/.../Repository/OrderRepositoryDapper.cs Added a finally block to ensure _dbConnection is closed after transactions.
src/.../Repository/ProductRepositoryDapper.cs Added multiple new methods and a constructor to ProductRepositoryDapper for improved product management.
tests/.../Repository/ProductRepositoryDapperTest.cs Introduced a new test class for testing the CreateAsync method in ProductRepositoryDapper.

Recent Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 572b6ee and e972fd6.
Files selected for processing (6)
  • .docker/setup.sql (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/Controllers/ProductController.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Api/ServiceCollectionExtensions.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Infrastructure/Repository/OrderRepositoryDapper.cs (1 hunks)
  • src/FIAP.TechChallenge.ByteMeBurger.Infrastructure/Repository/ProductRepositoryDapper.cs (1 hunks)
  • tests/FIAP.TechChallenge.ByteMeBurger.Infrastructure.Test/Repository/ProductRepositoryDapperTest.cs (1 hunks)
Files skipped from review due to trivial changes (2)
  • .docker/setup.sql
  • src/FIAP.TechChallenge.ByteMeBurger.Api/ServiceCollectionExtensions.cs
Additional Context Used
GitHub Check Runs (1)
build success (10)

src/FIAP.TechChallenge.ByteMeBurger.Infrastructure/Repository/OrderRepositoryDapper.cs: [warning] 41-41:
The variable 'e' is declared but never used

Path-based Instructions (4)
tests/FIAP.TechChallenge.ByteMeBurger.Infrastructure.Test/Repository/ProductRepositoryDapperTest.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design principles and Ports & Adapters architecture pattern.

src/FIAP.TechChallenge.ByteMeBurger.Infrastructure/Repository/ProductRepositoryDapper.cs (1)

Pattern **/*.cs: Validate all files and structures against Domain Driven Design principles and Ports & Adapters architecture pattern.

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

Pattern **/*.cs: Validate all files and structures against Domain Driven Design principles and Ports & Adapters architecture pattern.

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

Pattern **/*.cs: Validate all files and structures against Domain Driven Design principles and Ports & Adapters architecture pattern.

Additional comments not posted (3)
tests/FIAP.TechChallenge.ByteMeBurger.Infrastructure.Test/Repository/ProductRepositoryDapperTest.cs (1)

17-23: Setup for unit tests looks appropriate and well-structured.

src/FIAP.TechChallenge.ByteMeBurger.Infrastructure/Repository/ProductRepositoryDapper.cs (1)

24-37: The CreateAsync method is correctly implemented with parameterized SQL queries to prevent SQL injection.

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

46-49: Proper use of transaction management and resource cleanup in the finally block enhances reliability.


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.

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 a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @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: 3

Out of diff range and nitpick comments (1)
src/FIAP.TechChallenge.ByteMeBurger.Infrastructure/Repository/OrderRepositoryDapper.cs (1)

Line range hint 41-41: The exception variable e is declared but never used. Consider removing it or using it for logging or more detailed error handling.

- catch (Exception e)
+ catch (Exception)

// Arrange
var product = new Product("product", "description", ProductCategory.Beverage, 10, ["image1"]);

_mockConnection.SetupDapperAsync(c => c.ExecuteAsync("", null, null, null, null))
Copy link

@coderabbitai coderabbitai bot May 1, 2024

Choose a reason for hiding this comment

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

The SQL command in the mock setup is an empty string. Replace it with the actual SQL command used in the repository to ensure the test is valid.

Copy link
Member Author

Choose a reason for hiding this comment

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

this mock is almost useless for dapper. I'm basically adding it to keep a good coverage rate

Copy link

Choose a reason for hiding this comment

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

@italopessoa, I understand the need to maintain a good coverage rate, and it's a common practice to include certain tests to achieve this metric. However, to enhance the test's value and ensure it more accurately reflects the production behavior, you might consider using actual SQL commands and parameters in your mock setups. This approach not only improves coverage qualitatively but also helps catch potential issues during the testing phase.

If you're open to it, I can suggest a more detailed example of how you might structure these tests to simulate more realistic database interactions. This could involve using actual SQL queries and verifying that the interactions with the database occur as expected, which could provide more confidence in the behavior of your application under test conditions.

Would you like to explore this further, or do you have specific constraints that require sticking to the current testing approach?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, let's explore it more. give me one better example

Copy link

Choose a reason for hiding this comment

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

@italopessoa, here's a more detailed example of how you might structure the unit test for the CreateAsync method in the ProductRepositoryDapper class to simulate more realistic database interactions:

[Fact]
public async Task Create_Success()
{
    // Arrange
    var product = new Product("product", "description", ProductCategory.Beverage, 10, new List<string> { "image1" });
    var sql = "INSERT INTO Products (Name, Description, Category, Price, Images) VALUES (@Name, @Description, @Category, @Price, @Images)";
    var parameters = new { Name = product.Name, Description = product.Description, Category = product.Category.ToString(), Price = product.Price, Images = string.Join(",", product.Images) };

    _mockConnection.SetupDapperAsync(c => c.ExecuteAsync(sql, parameters, null, null, null))
                   .ReturnsAsync(1) // Assuming the execution of the SQL command returns 1 row affected
                   .Verifiable("The SQL command was not executed with the expected parameters.");

    // Act
    var result = await _target.CreateAsync(product);

    // Assert
    result.Should().NotBeNull();
    result.Should().BeEquivalentTo(product, options => options.ComparingByMembers<Product>());
    _mockConnection.Verify(); // Verify that the SQL command was executed with the correct parameters
}

In this example:

  • The SQL command for inserting a product is specified along with the expected parameters.
  • The SetupDapperAsync method is used to set up the expected behavior of the ExecuteAsync method when it is called with the specified SQL command and parameters.
  • The test checks that the method returns the correct product object and that the SQL command is executed with the correct parameters.

This approach not only helps in achieving a higher coverage rate but also ensures that the unit tests are valuable by verifying that the interactions with the database occur as expected. This can provide more confidence in the behavior of your application under test conditions.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll fix it, thanks. why are you using ComparingByMembers?

Copy link

Choose a reason for hiding this comment

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

@italopessoa, I'm glad to hear you'll be updating the test!

Regarding your question about using ComparingByMembers in the test: This method is used with FluentAssertions to specify that when comparing two objects, the comparison should be done based on the members (properties) of the objects rather than their references.

In unit tests, especially when dealing with entity objects like Product, it's common to create an instance of an object with certain properties, perform operations (like saving to a database), and then retrieve it or receive a copy. The two instances of the object are different in memory (different references), but they are considered equivalent if all their properties match.

Here's why it's useful in your test:

  • Accuracy: Ensures that the product returned by CreateAsync method is exactly what you expect in terms of data. It checks every property of the Product object to match with what was intended to be saved.
  • Robustness: Makes the test more robust by verifying that all properties are correctly handled by the CreateAsync method, not just that the method returns a non-null or any Product object.

This approach is particularly beneficial in testing methods that involve data manipulation or transformation, ensuring that no property values are altered unexpectedly during the process.

If you have any more questions or need further assistance, feel free to ask!

Comment on lines +19 to +21
public Task<Product?> FindByIdAsync(Guid id)
{
throw new NotImplementedException();
Copy link

@coderabbitai coderabbitai bot May 1, 2024

Choose a reason for hiding this comment

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

The FindByIdAsync method is not implemented and needs to be completed.

Copy link
Member Author

Choose a reason for hiding this comment

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

that'll will be worked on issue #32 could you add the description to it?

Copy link

Choose a reason for hiding this comment

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

The current description of issue #32 is a general template. I will add specific details about the implementation of the FindByIdAsync method. Here's the proposed update:

Is your feature request related to a problem? Please describe.
The FindByIdAsync method in ProductRepositoryDapper is currently not implemented, which is essential for retrieving product details by their ID.

Describe the solution you'd like
Implement the FindByIdAsync method to enable fetching of product details from the database using the Dapper ORM. This method should handle any potential exceptions and ensure the connection is properly managed.

Describe alternatives you've considered
Not applicable as the method implementation is crucial for the repository's functionality.

Additional context
This method's implementation is critical for the product management feature, allowing detailed queries on individual products.

Please confirm if this update looks good, and I will proceed to update the issue on GitHub.

Analysis chain
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Check if issue #32 exists and fetch its current description
gh issue view 32 --json body -R soat-fiap/FIAP.TechChallenge.ByteMeBurger

Length of output: 608

Copy link
Member Author

Choose a reason for hiding this comment

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

@coderabbitai generate unit testing code for this file.

Copy link

Choose a reason for hiding this comment

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

using System;
using System.Collections.ObjectModel;
using System.Data;
using System.Threading.Tasks;
using Dapper;
using FIAP.TechChallenge.ByteMeBurger.Domain.Entities;
using FIAP.TechChallenge.ByteMeBurger.Domain.ValueObjects;
using FIAP.TechChallenge.ByteMeBurger.Infrastructure.Repository;
using FluentAssertions;
using Moq;
using Xunit;

namespace FIAP.TechChallenge.ByteMeBurger.Infrastructure.Test.Repository
{
    public class ProductRepositoryDapperTest
    {
        private readonly Mock<IDbConnection> _mockDbConnection;
        private readonly ProductRepositoryDapper _repository;

        public ProductRepositoryDapperTest()
        {
            _mockDbConnection = new Mock<IDbConnection>();
            _repository = new ProductRepositoryDapper(_mockDbConnection.Object);
        }

        [Fact]
        public async Task CreateAsync_Success()
        {
            // Arrange
            var product = new Product(Guid.NewGuid(), "Burger", ProductCategory.Food, new Price(5.99m));
            _mockDbConnection.Setup(db => db.ExecuteAsync(It.IsAny<string>(), It.IsAny<object>(), null, null, null)).ReturnsAsync(1);

            // Act
            var result = await _repository.CreateAsync(product);

            // Assert
            result.Should().NotBeNull();
            result.Id.Should().Be(product.Id);
        }

        [Fact]
        public async Task DeleteAsync_Success()
        {
            // Arrange
            var productId = Guid.NewGuid();
            _mockDbConnection.Setup(db => db.ExecuteAsync(It.IsAny<string>(), It.IsAny<object>(), null, null, null)).ReturnsAsync(1);

            // Act
            var result = await _repository.DeleteAsync(productId);

            // Assert
            result.Should().BeTrue();
        }

        [Fact]
        public async Task GetAll_Success()
        {
            // Arrange
            var products = new ReadOnlyCollection<Product>(new List<Product>
            {
                new Product(Guid.NewGuid(), "Burger", ProductCategory.Food, new Price(5.99m)),
                new Product(Guid.NewGuid(), "Fries", ProductCategory.Food, new Price(2.99m))
            });
            _mockDbConnection.Setup(db => db.QueryAsync<Product>(It.IsAny<string>(), null, null, null, null)).ReturnsAsync(products);

            // Act
            var result = await _repository.GetAll();

            // Assert
            result.Should().BeEquivalentTo(products);
        }

        [Fact]
        public async Task FindByCategory_Success()
        {
            // Arrange
            var category = ProductCategory.Food;
            var products = new ReadOnlyCollection<Product>(new List<Product>
            {
                new Product(Guid.NewGuid(), "Burger", category, new Price(5.99m)),
                new Product(Guid.NewGuid(), "Fries", category, new Price(2.99m))
            });
            _mockDbConnection.Setup(db => db.QueryAsync<Product>(It.IsAny<string>(), It.IsAny<object>(), null, null, null)).ReturnsAsync(products);

            // Act
            var result = await _repository.FindByCategory(category);

            // Assert
            result.Should().BeEquivalentTo(products);
        }

        [Fact]
        public async Task UpdateAsync_Success()
        {
            // Arrange
            var product = new Product(Guid.NewGuid(), "Burger", ProductCategory.Food, new Price(6.99m));
            _mockDbConnection.Setup(db => db.ExecuteAsync(It.IsAny<string>(), It.IsAny<object>(), null, null, null)).ReturnsAsync(1);

            // Act
            var result = await _repository.UpdateAsync(product);

            // Assert
            result.Should().BeTrue();
        }

        [Fact]
        public async Task FindByIdAsync_NotImplemented()
        {
            // Act & Assert
            await Assert.ThrowsAsync<NotImplementedException>(() => _repository.FindByIdAsync(Guid.NewGuid()));
        }
    }
}

@italopessoa, I've generated a comprehensive set of unit tests for the ProductRepositoryDapper class. These tests cover the CRUD operations and the constructor, using Moq for mocking the database connection and FluentAssertions for assertions. The FindByIdAsync method test is set to expect a NotImplementedException as it's not yet implemented. Let me know if there's anything else you need!

Copy link
Member Author

Choose a reason for hiding this comment

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

the update looks good, please proceed to the github issue

Copy link

Choose a reason for hiding this comment

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

@italopessoa, the description for issue #32 has been successfully updated as requested. You can view the updated issue here: GitHub Issue #32. If there's anything else you need, feel free to let me know!

Analysis chain
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Update GitHub issue #32 with the new description
gh issue edit 32 --body "Is your feature request related to a problem? Please describe.\nThe \`FindByIdAsync\` method in \`ProductRepositoryDapper\` is currently not implemented, which is essential for retrieving product details by their ID.\n\nDescribe the solution you'd like\nImplement the \`FindByIdAsync\` method to enable fetching of product details from the database using the Dapper ORM. This method should handle any potential exceptions and ensure the connection is properly managed.\n\nDescribe alternatives you've considered\nNot applicable as the method implementation is crucial for the repository's functionality.\n\nAdditional context\nThis method's implementation is critical for the product management feature, allowing detailed queries on individual products." -R soat-fiap/FIAP.TechChallenge.ByteMeBurger

Length of output: 903


If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

Copy link
Member Author

Choose a reason for hiding this comment

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

since I'm using Mod.Dapper, the correct method to use should be SetupDapperAsync instead of Setup

Copy link

Choose a reason for hiding this comment

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

@italopessoa, based on your feedback regarding the use of SetupDapperAsync instead of Setup for the Mod.Dapper library, I've adjusted the unit test code accordingly. Here's the updated version:

using System;
using System.Collections.ObjectModel;
using System.Data;
using System.Threading.Tasks;
using Dapper;
using FIAP.TechChallenge.ByteMeBurger.Domain.Entities;
using FIAP.TechChallenge.ByteMeBurger.Domain.ValueObjects;
using FIAP.TechChallenge.ByteMeBurger.Infrastructure.Repository;
using FluentAssertions;
using Moq;
using Xunit;

namespace FIAP.TechChallenge.ByteMeBurger.Infrastructure.Test.Repository
{
    public class ProductRepositoryDapperTest
    {
        private readonly Mock<IDbConnection> _mockDbConnection;
        private readonly ProductRepositoryDapper _repository;

        public ProductRepositoryDapperTest()
        {
            _mockDbConnection = new Mock<IDbConnection>();
            _repository = new ProductRepositoryDapper(_mockDbConnection.Object);
        }

        [Fact]
        public async Task CreateAsync_Success()
        {
            // Arrange
            var product = new Product(Guid.NewGuid(), "Burger", ProductCategory.Food, new Price(5.99m));
            _mockDbConnection.SetupDapperAsync(db => db.ExecuteAsync(It.IsAny<string>(), It.IsAny<object>(), null, null, null)).ReturnsAsync(1);

            // Act
            var result = await _repository.CreateAsync(product);

            // Assert
            result.Should().NotBeNull();
            result.Id.Should().Be(product.Id);
        }

        [Fact]
        public async Task DeleteAsync_Success()
        {
            // Arrange
            var productId = Guid.NewGuid();
            _mockDbConnection.SetupDapperAsync(db => db.ExecuteAsync(It.IsAny<string>(), It.IsAny<object>(), null, null, null)).ReturnsAsync(1);

            // Act
            var result = await _repository.DeleteAsync(productId);

            // Assert
            result.Should().BeTrue();
        }

        [Fact]
        public async Task GetAll_Success()
        {
            // Arrange
            var products = new ReadOnlyCollection<Product>(new List<Product>
            {
                new Product(Guid.NewGuid(), "Burger", ProductCategory.Food, new Price(5.99m)),
                new Product(Guid.NewGuid(), "Fries", ProductCategory.Food, new Price(2.99m))
            });
            _mockDbConnection.SetupDapperAsync(db => db.QueryAsync<Product>(It.IsAny<string>(), null, null, null, null)).ReturnsAsync(products);

            // Act
            var result = await _repository.GetAll();

            // Assert
            result.Should().BeEquivalentTo(products);
        }

        [Fact]
        public async Task FindByCategory_Success()
        {
            // Arrange
            var category = ProductCategory.Food;
            var products = new ReadOnlyCollection<Product>(new List<Product>
            {
                new Product(Guid.NewGuid(), "Burger", category, new Price(5.99m)),
                new Product(Guid.NewGuid(), "Fries", category, new Price(2.99m))
            });
            _mockDbConnection.SetupDapperAsync(db => db.QueryAsync<Product>(It.IsAny<string>(), It.IsAny<object>(), null, null, null)).ReturnsAsync(products);

            // Act
            var result = await _repository.FindByCategory(category);

            // Assert
            result.Should().BeEquivalentTo(products);
        }

        [Fact]
        public async Task UpdateAsync_Success()
        {
            // Arrange
            var product = new Product(Guid.NewGuid(), "Burger", ProductCategory.Food, new Price(6.99m));
            _mockDbConnection.SetupDapperAsync(db => db.ExecuteAsync(It.IsAny<string>(), It.IsAny<object>(), null, null, null)).ReturnsAsync(1);

            // Act
            var result = await _repository.UpdateAsync(product);

            // Assert
            result.Should().BeTrue();
        }

        [Fact]
        public async Task FindByIdAsync_NotImplemented()
        {
            // Act & Assert
            await Assert.ThrowsAsync<NotImplementedException>(() => _repository.FindByIdAsync(Guid.NewGuid()));
        }
    }
}

This updated code uses SetupDapperAsync for setting up the mock expectations, which should align with your usage of the Mod.Dapper library. Please review and let me know if further adjustments are needed!

@italopessoa italopessoa merged commit e40825f into main May 2, 2024
1 check passed
@italopessoa italopessoa deleted the 3-add-new-endpoint-to-register-new-products branch May 2, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Phase 1 Containers in Software Architecture
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add new endpoint to register new Products
1 participant