-
Notifications
You must be signed in to change notification settings - Fork 72
Project finished #86
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?
Project finished #86
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 @Lepros311 👋,
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 👍
|
|
||
| public DbSet<LineItem> LineItems { get; set; } | ||
|
|
||
| protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=EcommerceDb;Trusted_Connection=True;Initial Catalog=EcommerceDb"); |
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.
🟢 SQL Server
| using Ecommerce.Api.Repository; | ||
| using Ecommerce.Api.Services; | ||
|
|
||
| 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
|
|
||
| namespace Ecommerce.Api.Data; | ||
|
|
||
| public class EcommerceDbContext : DbContext |
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.
🟢 Products, Categories and Sales
|
|
||
| protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=EcommerceDb;Trusted_Connection=True;Initial Catalog=EcommerceDb"); | ||
|
|
||
| protected override void OnModelCreating(ModelBuilder modelBuilder) |
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.
🟢 Products and Sales Many-to-Many Relationship
|
|
||
| public string ProductName { get; set; } | ||
|
|
||
| public decimal Price { get; set; } |
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.
🟢 Products need to have a price
| } | ||
|
|
||
| [HttpGet] | ||
| public async Task<ActionResult<List<ProductDto>>> GetPagedProducts([FromQuery] PaginationParams paginationParams) |
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.
🟢 GetProducts endpoints need to have pagination capabilities
| } | ||
|
|
||
| [HttpGet] | ||
| public async Task<ActionResult<List<CategoryDto>>> GetPagedSales([FromQuery] PaginationParams paginationParams) |
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.
🟢 GetSales endpoints need to have pagination capabilities
|
|
||
| public bool IsDeleted { get; set; } = false; | ||
|
|
||
| public override string ToString() => $"{CategoryName}"; |
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.
🟡 String Interpolation
❓ Any need here? surely this will do:
| public override string ToString() => $"{CategoryName}"; | |
| public override string ToString() => CategoryName; |
| } | ||
| }, | ||
| "AllowedHosts": "*", | ||
| "ConnectionStrings": { "DefaultConnection": "Server=(localdb)\\mssqllocaldb;Database=EcommerceDb;Trusted_Connection=True;Initial Catalog=shiftsDb" } |
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.
🟠 Environment Variables
💡 You really need to make your mind up whether you are going to use this appsettings connection string, or the hard coded one you provide to OnConfiguring...
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 File
No description provided.