Igor Magalhaes Week 4 IC #11
Conversation
…s, feat: view with recipe name, description, total calories and author
feat: form to add ingredient to recipe
delete button to dish ingredient
| redirect_to dish_path(params[:id]) | ||
| end | ||
|
|
||
| def destroy |
There was a problem hiding this comment.
Nice work making it to the extension! Great use of find_by
| @@ -0,0 +1,6 @@ | |||
| class IngredientsController < ApplicationController | |||
| def index | |||
There was a problem hiding this comment.
This is an okay controller option for US 3, but you could also consider a nested controller: chefs/ingredients_controller since this is a list of ingredients just for 1 specific chef
| @@ -1,4 +1,5 @@ | |||
| class Chef < ApplicationRecord | |||
| validates_presence_of :name | |||
| has_many :dishes | |||
| has_many :ingredients, through: :dishes | |||
There was a problem hiding this comment.
Even though distinct makes a small difference on the query, by pure MVC principles, it's beyond the basic list of CRUD methods in AR so it could go in a model method here. Nice work getting the additional association to ingredients to make it easier!
def ingredient_list
self.ingredients.distinct.pluck(:name) # plucking is optional
end
| has_many :ingredients, through: :ingredient_dishes | ||
|
|
||
| def total_calories | ||
| ingredients.sum(:calories) |
| <% @dish.ingredients.each do |ingredient| %> | ||
| <div id="ingredient_<%= ingredient.id %>"> | ||
| <li><%= ingredient.name %></li> | ||
| <%= form_with model: @dish, url: destroy_ingredient_dish_path(@dish), method: :delete do |form| %> |
There was a problem hiding this comment.
form_with model is just another way to set up your forms, but I generally think it's best only when you're using it to create or edit a whole object, rather than doing something like creating a joins record. It can sometimes complicate things more for forms where you're doing something different (like in this case) so just be careful! You don't have to use it, it's just another tool in your toolbox
| delete "/dishes/:id/ingredients", to: "ingredient_dishes#destroy", as: :destroy_ingredient_dish | ||
|
|
||
| resources :chefs, only: [:show] do | ||
| resources :ingredients, only: [:index] |
There was a problem hiding this comment.
Nice RESTful routes! For this one, if you wanted to direct this route to a nested controller, you could add controller: "chefs/ingredients" to the end of line 12
| @@ -8,6 +8,7 @@ | |||
|
|
|||
| describe "relationships" do | |||
| it {should have_many :dishes} | |||
| it {should have_many(:ingredients).through(:dishes)} | |||
There was a problem hiding this comment.
Great work remembering to add a test for this new association!
Hey guys, I had a good time doing this over the weekend and forgot to submit it yesterday. I'd appreciate if I could get some feedback. Thank you!