Completed project #81
Conversation
Project Set-Up
There was a problem hiding this comment.
Overall well done, the site looks great and has optional functionality. However the API Wrapper was a bit sloppily implemented mixing with the controller and of course testing wasn't done.
Overall it's good, but a few things to finish before turning this into a great portfolio piece.
|
|
||
| def signed_in? | ||
| !!current_user | ||
| end |
| @user = user | ||
| session[:user_id] = user.id | ||
| end | ||
| end |
| session[:search_terms].shift | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
nice a maximum of 5 remembered search terms!
| @recent_search_terms = session[:search_terms] | ||
|
|
||
| @recipes = EdamamApiWrapper.search_results(params[:q], params[:from], params[:to], params["health1"], params["health2"], params["diet1"]) | ||
|
|
There was a problem hiding this comment.
You should create a Recipe class which encapsulates the concept of a recipe and lets you separate your controller from the nitty-gritty details of the API's implementation.
Similarly to how we did the Slack Channel.
| @@ -0,0 +1,52 @@ | |||
| require 'httparty' | |||
There was a problem hiding this comment.
It would be better to create a Recipe class to encapsulate the Recipe concept which lets your controller not have to be concerned with the details of the API.
| @@ -0,0 +1,39 @@ | |||
| require 'test_helper' | |||
There was a problem hiding this comment.
Few passing tests & no api testing.
| @@ -0,0 +1,208 @@ | |||
| /* | |||
There was a problem hiding this comment.
could be a little better organized, but that's knitpicking.
| @@ -0,0 +1,86 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
I would suggest using a bit more semantic HTML in your views rather than wrapping everything in divs. However that's a minor criticism.
| get '/favorites', to: 'pages#favorites', as: 'favorites' | ||
|
|
||
| resources :recipes, :sessions | ||
|
|
I would still like to deploy to Heroku.
I still have to write tests.