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

Feature 25 edit options #93

Closed
wants to merge 23 commits into from
Closed

Feature 25 edit options #93

wants to merge 23 commits into from

Conversation

YannisAm
Copy link
Contributor

@YannisAm YannisAm commented Apr 4, 2021

closes #25

Copy link
Collaborator

@chrisK00 chrisK00 left a comment

Choose a reason for hiding this comment

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

Good job! Just need to look up automapper.

Automapper docs are very good https://docs.automapper.org/en/stable/Getting-started.html

@@ -70,6 +70,19 @@ public async Task<string> AddIngredient(IngredientAddDTO ingredientAddDTO)
return ingredientAddDTO.Name;
}

public async Task UpdateIngredient(string name, IngredientAddDTO ingredientAddDTO)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@@ -78,6 +78,19 @@ public async Task<string> AddIngredientToRecipe(IngredientAddDTO ingredientAddDT
return recipeName;
}

public async Task UpdateRecipe(string name, RecipeAddDTO recipeAddDTO)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I recommend doing it the way we did with automapper in the ingredientservice. Msg me on discord if you want me to explain if it isnt clear whats happening

// this is super shit code, sorry
//TODO: Fix this mess
var recipe = await _recipeService.GetRecipeByName(name);
RecipeAddDTO recipeAdd = new RecipeAddDTO() { Name = recipe.Name };
Copy link
Collaborator

Choose a reason for hiding this comment

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

Automapper can fix this easily

  1. We need a configuration inside the AutoMapperProfiles CreateMap<from, to>
  2. use the mapper like this: _mapper.Map(fromObject)

}
else if (char.ToUpper(editWhat[0]) == 'I')
{
//TODO: Not DRY, extract this and use it in common locations
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good idea, some sort of generic method or factory is probably the way to go.

@YannisAm YannisAm closed this Apr 11, 2021
@YannisAm YannisAm reopened this Apr 16, 2021
@YannisAm YannisAm closed this Apr 16, 2021
@YannisAm YannisAm deleted the feature-25-editOptions branch April 16, 2021 16:34
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 the ability to edit recipes and ingredients from the console menu
5 participants