-
Notifications
You must be signed in to change notification settings - Fork 72
Project Completed #87
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
base: main
Are you sure you want to change the base?
Conversation
chrisjamiecarter
left a comment
There was a problem hiding this 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 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟢 README
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dependency Injection
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 Recommend Removal
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟢 Pagination
No description provided.