Skip to content
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

Conversation

AlakaTahir
Copy link
Contributor

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.

  • My code follows the code style of this project.
  • This PR does not contain plagiarized content.
  • The title and description of the PR is clear and explains the approach.
  • I am making a pull request against the dev branch (left side).
  • My commit messages styles matches our requested structure.
  • My code additions will fail neither code linting checks nor unit test.
  • I am only making changes to files I was requested to.

Screenshots/Videos

Copy link
Contributor

@chimaihueze chimaihueze left a 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.

Copy link
Contributor

@chimaihueze chimaihueze left a 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;
Copy link
Contributor

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)
{
Copy link
Contributor

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.

@Godhanded Godhanded changed the title Feature/add multiple products main dashboard Feat(Dashboard): add multiple products in main dashboard Aug 6, 2024
{
public class AddProductsCommand : IRequest<ProductsDto>
{
public AddProductsCommand(string userId, List<ProductCreationDto> productCreation)
Copy link
Contributor

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

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

@AlakaTahir AlakaTahir closed this Aug 8, 2024
@AlakaTahir
Copy link
Contributor Author

Another branch created

@AlakaTahir AlakaTahir deleted the feature/add-multiple-products-main-dashboard branch August 8, 2024 09:35
@AlakaTahir AlakaTahir restored the feature/add-multiple-products-main-dashboard branch August 8, 2024 09:35
@AlakaTahir AlakaTahir deleted the feature/add-multiple-products-main-dashboard branch August 8, 2024 09:35
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.

Add products (main dashboard)
3 participants