-
Notifications
You must be signed in to change notification settings - Fork 42
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
Feat(Dashboard): add multiple products in main dashboard #219
Feat(Dashboard): add multiple products in main dashboard #219
Conversation
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.
follow the PR naming standards, and write a detailed description for your PR.
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.
Write your unit tests as well.
[ProducesResponseType(typeof(ProductDto), StatusCodes.Status201Created)] | ||
public async Task<ActionResult<ProductsDto>> AddProducts([FromBody] AddMultipleProductDto body) | ||
{ | ||
var loggedInUserId = HttpContext.User.FindFirst(ClaimTypes.Sid).Value; |
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.
There is a service that already does this. Check the AuthenticationService in Infrastructure. You should move whatever logic you want to perform with this to the handler. Keep this controller simple.
public class AddProductsCommand : IRequest<ProductsDto> | ||
{ | ||
public AddProductsCommand(string userId, List<ProductCreationDto> productCreation) | ||
{ |
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.
This command is for adding Product, it has no business taking userId as a parameter/argument. If you want to assign a user to a product. Do it in the handler.
{ | ||
public class AddProductsCommand : IRequest<ProductsDto> | ||
{ | ||
public AddProductsCommand(string userId, List<ProductCreationDto> productCreation) |
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.
don't collect the userId from the request, use the getloggedinuserId function in authentication service class in hng.infrastructure
{ | ||
var product = _mapper.Map<Product>(item); | ||
product.Id = Guid.NewGuid(); | ||
product.UserId = Guid.Parse(request.UserId); |
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.
use the GetLoggedInUserId function in hng.infrastructure/services/authenticationservice to get the id of the user
Another branch created |
Description
Closes #218
Changes proposed
What were you told to do?
I added endpoint to add multiple products
Check List (Check all the applicable boxes)
🚨Please review the contribution guideline for this repository.
Screenshots/Videos