Skip to content

Project Completed#87

Open
GoldRino456 wants to merge 4 commits intothe-csharp-academy:mainfrom
GoldRino456:main
Open

Project Completed#87
GoldRino456 wants to merge 4 commits intothe-csharp-academy:mainfrom
GoldRino456:main

Conversation

@GoldRino456
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@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 👍

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 README

Copy link
Copy Markdown

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);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Dependency Injection

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Recommend Removal

Copy link
Copy Markdown

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();
Copy link
Copy Markdown

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.

Copy link
Copy Markdown

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