Skip to content

Conversation

@GoldRino456
Copy link

No description provided.

Copy link

@chrisjamiecarter chrisjamiecarter left a comment

Choose a reason for hiding this comment

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

Hey @GoldRino456 👋,

Excellent work on your Ecommerce API project submission 🎉!

I have performed a peer review. Review/ignore any comments as you wish.


🟢 Requirements

⭐ You have fulfilled all of the project requirements!


I will go ahead and mark as approved, keep up the excellent work on the next projects! 😊

Best regards,
@chrisjamiecarter 👍

Choose a reason for hiding this comment

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

🟢 README

Choose a reason for hiding this comment

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

🟢 Postman Collection

using Microsoft.EntityFrameworkCore;
using System.Text.Json.Serialization;

var builder = WebApplication.CreateBuilder(args);

Choose a reason for hiding this comment

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

Dependency Injection

Choose a reason for hiding this comment

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

🟠 Recommend Removal

Choose a reason for hiding this comment

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

🟢 Filtering

[HttpGet]
public async Task<ActionResult<PagedResponse<GameProductResponseDto>>> GetAllGamesAsync([FromQuery] PaginationParams pagination, [FromQuery] GameProductFilterParams filters)
{
var query = _gamesService.GetAllGames();

Choose a reason for hiding this comment

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

🟠 Not in Controller

💡 I would recommend you pass the responsibility down to your service, then repo. Currently if you filter to 1 record, you get all from the database, and then filter in memory. EFCore can do all this for you.

Choose a reason for hiding this comment

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

🟢 Pagination

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.

2 participants