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

Kay api-muncher #28

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

Kay api-muncher #28

wants to merge 14 commits into from

Conversation

kayxn23
Copy link

@kayxn23 kayxn23 commented Nov 5, 2018

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?
By first reading the documentation of the API, and then using Postman to test out the methods.

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)? |
Using an API wrapper allows us to separate our concerns so that all the API specific stuff happens in the one class called the API wrapper so if we switch from Slack to using Microsoft we can just switch classes and the rest of our app won’t have to change. It lets us adapt our code more easily. It lives in the 'lib' folder because that is where custom libraries live, regular classes would live in the model folder.

Describe your API wrapper, the methods you created in it, and why you decided to create those methods/how they were helpful | The API wrapper has three methods: self.list_recipes(user_search), self.find_recipe_by(uri), and a private method self.create_recipe(api_params). The #create_recipes is an important private method that takes the api_params from Edamam and returns an instance of Recipe. Rather than creating an instance of Recipe directly from the 'uri' value that comes from api_params, I partitioned the uri string to only select the unique recipe identifier that comes at the end of the full uri. The #find_recipe_by(uri) method searches for a specific recipe by its uri, calls #create_recipe helper method and returns an instance of Recipe containing the attributes of the searched recipe. Reading the Edimam documentation was important because it specified that you can search for uri by specifiying "r=" in the url. That method is called in #show in the Recipes Controller. The #list_recipes(user_search) searches for all recipes in the API's database, "q=" needed to be speficied in the url, it calls the #create_recipe helper for each recipe and returns and array of Recipe objects. This method is called in the #index in Recipes Controller. Is it necessary to call 'encode.URI' in these two methods? I found that it broke the app at one point when I used it so I commented it out.

What was an edge case or failure case test you wrote for your API Wrapper? What was a nominal case? | Two edge cases are "it returns empty array when given a search term that doesn't exist", "returns nil if non-existing uri is passed". A nominal case is it "can list recipes when given a search term that exists in the API"
How does VCR aid in testing an API? | It records the interaction with the API and lets you replay it later so you don’t have to make too many API calls.
What is the Heroku URL of your deployed application? |
https://kay-api-muncher.herokuapp.com/

@dHelmgren
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good
Comprehension questions Answers are good, but hard to read. Don't forget to preview your comprehension questions before you hit submit
General
Rails fundamentals (RESTful routing, use of named paths) good, but see comments
Semantic HTML Good
Errors are reported to the user Yes
API Wrapper to handle the API requests yes
Controller testing good
Lib testing good
Search Functionality Good
List Functionality yes
Show individual item functionality yes
Styling
List view shows 10 items at a time and/or has pagination yes
Attractive and usable UI Site looks good, but I don't think that font is very easy to read.
API Features
The App attributes Edamam Not on the show page.
The VCR cassettes do not contain the API key It's in a comment :( Do a Control+F to catch these before you sub!
External Resources
Link to deployed app on Heroku Yes
Overall This is a solid assignment, and I feel like you met the learning goals for the assignment!



#list the channels **
# base_url = https://api.edamam.com/search?app_id=cce9a91f&app_key=d0f4da3c9d8a64a9be87b192561284b7&q=chocolate mouse

Choose a reason for hiding this comment

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

You did. Such a good job. Keeping your app key hidden. Except for the comments. 😦

get '/recipes', to: 'recipes#index', as: 'search'
# get '/recipes/:url', to: 'recipes#show', as: 'recipe_url'

get '/recipes/:uri', to: 'recipes#show', as: 'recipe'

Choose a reason for hiding this comment

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

Could you use resources for the index and show pages?

def show
# binding.pry
@recipe = EdamamApiWrapper.find_recipe_by(params[:uri])
if @recipe.nil?

Choose a reason for hiding this comment

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

You've written your API wrapper to raise an error if the API call fails, but you're not looking for an error here. You should wrap this call in a begin/rescue.


must_respond_with :ok
end
end

Choose a reason for hiding this comment

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

What if you give them a negative page?

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