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.

Sign up for GitHub

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

Jump to bottom

feat: delete product from database #33

Merged
merged 1 commit into from
May 2, 2024
Merged

feat: delete product from database #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/FIAP.TechChallenge.ByteMeBurger.Infrastructure/Repository/ProductRepositoryDapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@ public async Task<Product> CreateAsync(Product product)
return product;
}

public Task<bool> DeleteAsync(Guid productId)
public async Task<bool> DeleteAsync(Guid productId)
{
throw new NotImplementedException();
var affectedRows = await _dbConnection.ExecuteAsync("delete from Products where Id = @Id;",
new { Id = productId });
return affectedRows == 1;
Comment on lines +40 to +44
Copy link

@coderabbitai coderabbitai bot May 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Implement error handling and logging for the DeleteAsync method.

While the method correctly executes the deletion and checks the number of affected rows, it currently lacks any form of error handling or logging. This could lead to situations where errors during the database operation are silently ignored, making debugging difficult. Consider wrapping the database operation in a try-catch block and logging the error before rethrowing or handling it appropriately.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hey, thanks for the feedback, could you please Open a follow-up GitHub issue with details and the sample code?
Thanks

Copy link

Choose a reason for hiding this comment

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

@italopessoa, I've successfully created a follow-up GitHub issue to address the need for error handling and logging in the DeleteAsync method. You can review and adjust the details as necessary in the issue here. Thank you for facilitating this enhancement!

Analysis chain

Once the issue is created, I will provide the user with the issue number and URL for their reference.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Create a new GitHub issue with details and sample code for error handling in DeleteAsync method

gh issue create \
  --title "Implement error handling and logging in DeleteAsync method" \
  --body "In the `DeleteAsync` method of the `ProductRepositoryDapper` class, there is currently no error handling or logging implemented. This can lead to situations where errors during the database operation are silently ignored, making debugging difficult. It is crucial to wrap the database operation in a try-catch block and log the error before rethrowing or handling it appropriately.

### Suggested Enhancement
\`\`\`csharp
public async Task<bool> DeleteAsync(Guid productId)
{
    try
    {
        var affectedRows = await _dbConnection.ExecuteAsync(\"delete from Products where Id = @Id;\", new { Id = productId });
        return affectedRows == 1;
    }
    catch (Exception ex)
    {
        // Log the exception details here
        throw; // Rethrow the exception or handle it as needed
    }
}
\`\`\`

### Context
This issue is linked to PR #33 and the discussion can be found [here](https://github.com/soat-fiap/FIAP.TechChallenge.ByteMeBurger/pull/33#discussion_r1587674739).

@italopessoa, please review the details and adjust as necessary." \
  --repo soat-fiap/FIAP.TechChallenge.ByteMeBurger

Length of output: 1432

}

public Task<ReadOnlyCollection<Product>> GetAll()
Expand Down
45 changes: 42 additions & 3 deletions ....TechChallenge.ByteMeBurger.Infrastructure.Test/Repository/ProductRepositoryDapperTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,17 @@ public ProductRepositoryDapperTest()
public async Task Create_Success()
{
// Arrange
var product = new Product("product", "description", ProductCategory.Beverage, 10, ["image1"]);
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("", null, null, null, null))
_mockConnection.SetupDapperAsync(c => c.ExecuteAsync(sql, parameters, null, null, null))
.ReturnsAsync(1);

// Act
Expand All @@ -39,7 +47,38 @@ public async Task Create_Success()
using (new AssertionScope())
{
result.Should().NotBeNull();
result.Should().Be(product);
result.Should().BeEquivalentTo(product, options => options.ComparingByMembers<Product>());
_mockConnection.Verify();
}
}

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

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

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

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

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

// Assert
result.Should().BeFalse();
}
}
Loading