-
Notifications
You must be signed in to change notification settings - Fork 39
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
Queues - Kayla Kubicke - API-Muncher #60
base: master
Are you sure you want to change the base?
Conversation
Updated recipe test to utilize let to make test objects. Changed recipe attributes to conform to project. Cleaned up redundant code in test.
Added test to evaluate unsuccessful search term result. Added logic and message for redirect if unsuccessful search term is entered.
Search tests
…t of site. Show page displays name, list of ingredients, and link to original page. Library and controller pass along parameters and create variables correctly for show page. Updated concept of site and added easter egg.
show-recipe
Removed incorrect attribute tests. Added edge cases for both methods.
A valid uri returns a recipe show page and an invalid one redirects. Empty search term redirects to root page.
…d root_path redirects in controller tests.
Previous next
Foundation 2
Decided to go more towards a sarcastic 70's hair ad vibe.
API MuncherWhat We're Looking For
Good work overall! Make sure to check out the inline comments - mostly small style things that don't fit up here. |
</head> | ||
<div class='row'> | ||
<div class="style1" align='center' height='35'> | ||
|
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.
All this stuff should be nested inside the <body>
tag.
@@ -0,0 +1,16 @@ | |||
<div class='row nav'> |
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.
I like that you've moved this code into a partial.
</div> <!-- These logic is here to keep the spacing in the page correct. --> | ||
<% else %> | ||
<div class='small-4 column' id='left-nav'> | ||
<%= link_to "Previous Recipes", recipes_path(:utf8 => params[:utf8], :search => params[:search], :page => (params[:page].to_i - 1), :commit => params[:commit]) %> |
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.
While this code is certainly functional, it's also a little clunky. Is there any way you could use a helper method here to clean it up?
lib/recipe.rb
Outdated
recipe_components[:ingredients] = response[0]['ingredientLines'] | ||
recipe_components[:diet] = response[0]['dietLabels'] | ||
|
||
return recipe_components |
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.
It's interesting to me that above you return Recipe
objects, but here you return a hash. Seems to me you could add a little bit of functionality to Recipe
and be able to reuse it nicely.
lib/recipe.rb
Outdated
end | ||
|
||
def self.show_recipe(uri) | ||
url = "https://api.edamam.com/search?app_key=#{ENV['APP_KEY']}&app_id=#{ENV['APP_ID']}&r=http://www.edamam.com/ontologies/edamam.owl%23#{uri[1..-1]}" |
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.
Both the base URL and the ontologies URL would be good candidates for class constants here.
Worked with Charles to DRY up Recipe Model and made method that exclusively creates Recipe objects. Adjusted tests to pass after the above changes, added tests for new method. Addressed most html comments, except navigation helper method.
API Muncher
Congratulations! You're submitting your assignment!
Comprehension Questions
I figured out a way to create page link navigation (i.e. Page 1, Page 2, etc.) without any empty pages. The response['count'] the API returns is always incorrect, it returns many blank recipes so it is an unreliable number to create links with. But, if you send a second request with the response['count'] as the last requested index, you receive a value in the next response called 'to' which returns the actual number of valid recipes. If you send both responses in tandem you can create page navigation without any empty page numbers. It's very slow but it does solve the problem of empty page links. This solution is on my 'pagination' branch.