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

Maryam - Api muncher #41

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

Maryam - Api muncher #41

wants to merge 15 commits into from

Conversation

marshi23
Copy link

@marshi23 marshi23 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? I used postman to get the right syntax for the requests and to see how the json was being returned.
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)? The API wrapper contains the logic to send and retrieve data from APIs. It is in the lib folder because it is a custom library we're creating for the app to use, we need to tell out app in config/application.rb to use liberaries we create in the lib folder. if we needed another wrapper, we can create another file for it in the lib folder.
Describe your API wrapper, the methods you created in it, and why you decided to create those methods/how they were helpful I have a method that creates recipes, create_recipe(api_params) that takes in incoming data and uses it to create an instance of a Recipe class, there is a search_recipes(arg) method that takes an argument that represents a food and searches Edamam's API for recipes that contain that food, and a find_recipe(uri) method that takes the uri of a returned list of recipes and requests it's details from the Edamam's API and returns the details of that recipe.
What was an edge case or failure case test you wrote for your API Wrapper? What was a nominal case? A nominal test was testing to see that the methods returns objects of the proper class, e.g .search_recipes(food) returns an array. Edge case example is that the .find_recipe(uri) method returns an errror if given an invalid uri.
How does VCR aid in testing an API? It records API responses the first time and saves that data for testing, it saves us from making too many request to the API we are using, potentially saving us money.
What is the Heroku URL of your deployed application? https://ghostmuncher.herokuapp.com/

@dHelmgren
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good
Comprehension questions good
General
Rails fundamentals (RESTful routing, use of named paths) yes
Semantic HTML yes
Errors are reported to the user yes
API Wrapper to handle the API requests yes
Controller testing Could be improved, see comments
Lib testing good work!
Search Functionality yes
List Functionality yes
Show individual item functionality yes
Styling
List view shows 10 items at a time and/or has pagination Yes, 10 per page, no pagination.
Attractive and usable UI mostly good, I would try to keep the images on the cards the same size.
API Features
The App attributes Edamam It doesn't follow their guidelines. They explicitly ask you to use their badges. This is really important! When you're designing code for money, using someone else's work without attribution is a good way to end up in legal trouble.
The VCR cassettes do not contain the API key No.
External Resources
Link to deployed app on Heroku yep
Overall This is a good submission that hits all our learning goals. Still the two things you need to keep in mind: Attribute code correctly and don't push you app secrets to github!

# For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html
root to: "recipes#home"
get "/recipes", to: "recipes#index", as: "recipes"
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?


uri = params[:uri]
@recipe = EdamamApiWrapper.find_recipe(uri)
rescue RecipeNotFoundError

Choose a reason for hiding this comment

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

nice job using rescue!

VCR.use_cassette('recipes') do
get recipes_path
must_respond_with :ok
end

Choose a reason for hiding this comment

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

There are some interesting cases you're not covering here:

  • What happens if no search term is provided?
  • What if the search returns no results?

It might also be worthwhile to add some tests around the paging parameters:

  • What happens if they're absent?
  • Do you get different results when they change?
  • What if you send a bogus value, like a negative page number?

http_interactions:
- request:
method: get
uri: https://api.edamam.com/search?app_id=<EDAMAM_TOKEN>&app_key=eede0a763580af2ed02e815f7904bbfd&r=http://www.edamam.com/ontologies/edamam.owl%23recipe_test_uri

Choose a reason for hiding this comment

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

:( I spy an App Key. :(

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