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

Christina Sherman Api-muncher #25

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

Conversation

Peacegypsy
Copy link

API Muncher

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How did you go about exploring the Edamam API, and how did you try querying the API? different search terms, parsing through the returned data.
What is an API Wrapper? Why is it in lib? How would your project change if you needed to interact with more than one API (aka more than just the Edamam API)? an API wrapper translates information between a controller and an API. We can store the API wrappers in a custom library in the lib folder. You would have to add additional config paths to load the additional paths.
Describe your API wrapper, the methods you created in it, and why you decided to create those methods/how they were helpful The wrapper has two search methods, one for a general search and one for a specific search. The third method is to create a recipe from the data returned from the search.
What was an edge case or failure case test you wrote for your API Wrapper? What was a nominal case? searching for an ingredient that was not a word , using non-sensical combinations of words.
How does VCR aid in testing an API? it allows us to rerun the tests multiple times without making multiple calls to the API. the cassette records the information and allows us to replay it for multiple tests.
What is the Heroku URL of your deployed application? I havent been able to deploy it to heroku yet.

@CheezItMan
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits and good commit messages
Comprehension questions Check, you didn't give a nominal case.
General
Rails fundamentals (RESTful routing, use of named paths) Check
Semantic HTML Some broken html, some limited use of section.
Errors are reported to the user Nope
API Wrapper to handle the API requests Check, but don't capitalize the file
Controller testing Some, but with a number of failing tests and errors in the tests.
Lib testing Similar to the above
Search Functionality Check
List Functionality Check
Show individual item functionality Unable to view
Styling
List view shows 10 items at a time and/or has pagination Nope
Attractive and usable UI it looks like you're unfinished so far, the recipes aren't linked from the search results, and no pagination.
API Features
The App attributes Edamam Nope
The VCR cassettes do not contain the API key Check
External Resources
Link to deployed app on Heroku Nope
Overall As you noted still a lot left to do. See my inline comments, I would like to see you finish this project as your final Rails web app. Biggest things to work on are, fixing your wrappers, and controllers and fixing your tests.

end
end

describe "show" do

Choose a reason for hiding this comment

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

This describe should be inside the describe RecipesController do block

it 'shows a list of recipes' do
VCR.use_cassette('recipes') do

get recipes_path, params: search

Choose a reason for hiding this comment

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

search is undefined, maybe this should be something like:

        get recipes_path, params: { search: 'pasta' }



it "can get the index path" do
ingredients = {ingredients: "chocolate"}

Choose a reason for hiding this comment

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

missing search key

must_respond_with :ok
end
end
it "will redirect with invalid search terms" do

Choose a reason for hiding this comment

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

This test crashes because of a bug in your index.html.erb view

if @search_term
@recipes = EdamamApiWrapper.search_recipes(params[:search])
else
@recipes = nil

Choose a reason for hiding this comment

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

maybe if @search_term is nil you should make @recipes be an empty array.

Otherwise your view gives an error when it tries to do .each on nil.

<section class="text-center">
<p class = "btn btn-outline-dark"><%= link_to "View Original Recipe ", "#{recipe.url}", target: :_blank %></p>
</section>
<div class="card-body">

Choose a reason for hiding this comment

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

You didn't close this div

end
return @recipe_list
else
return nil

Choose a reason for hiding this comment

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

I suggest returning an empty array if there are no results.

@@ -0,0 +1,53 @@
require 'httparty'

Choose a reason for hiding this comment

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

Your file should be lowercase.

def self.find_recipe(id)
url = BASE_URL + "search?" + "r=http%3A%2F%2Fwww.edamam.com%2Fontologies%2Fedamam.owl%23recipe_#{id}" + "&app_id=#{ID}&app_key=#{KEY}"
data = HTTParty.get(url)
if data.length

Choose a reason for hiding this comment

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

I would make this if data.length > 0

end

def show
@recipe = EdamamApiWrapper.find_recipe(recipe.id)

Choose a reason for hiding this comment

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

Shouldn't this be

@recipe = EdamamApiWrapper.find_recipe(params[:id])

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.

2 participants