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

API Muncher - Angela - Octos #31

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

Conversation

knockknockhusthere
Copy link

API Muncher

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How did you go about exploring the Edamam API, how did you try querying the API? I read through the documentation but mostly explored its structure using Postman.
Describe your API Wrapper. How did you decide on the methods you created? I had two public methods in my API wrapper. One queried Edamam for a list of recipes matching the search term, and the other queried Edamam for one specific recipe.
Describe an edge case or failure case test you wrote for your API Wrapper.
Explain how VCR aids in testing an API. VCR records API search results and allows these results to be re-used in testing. This saves on number of calls need to be made to the API, and proves useful if the API is down.
What is the Heroku URL of your deployed application? https://eatseatseats.herokuapp.com/
Provide a link to the Trello board you used https://trello.com/b/SwPxvG4Q/api-muncher

@droberts-sea
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene yes
Comprehension questions yes
General
Rails fundamentals (RESTful routing, use of named paths) yes
Semantic HTML yes
Errors are reported to the user no - when I mangle the recipe ID, I get an exception instead of a polite message
API Wrapper to handle the API requests yes
Controller testing some missing cases - see inline
Lib testing some missing cases - see inline
Search Functionality yes
List Functionality yes
Show individual item functionality (link to original recipe opens in new tab) yes (no link)
Styling
Responsive layout no
List View shows 10 items at a time/pagination yes
The app is styled to create an attractive user interface yes - black text on a dark background image is a little hard to read
API Features
The App attributes Edaman yes
The VCR casettes do not contain the API key yes
External Resources
Link to Trello Board yes
Link to deployed app on Heroku yes
Overall Good job overall. The core interactions with the API all appear to work just fine, which means the big learning goal for this assignment was met. I do see some room for improvement around error handling and test coverage - please review my inline comments below, and try to really focus on these topics in VideoStore API. Other than that I think things looks pretty good. Keep up the hard work!


get '/recipes', to: 'recipes#index', as: 'recipes'

get '/recipes/:label', 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 :recipes, only: [:index, :show] here?


if uri.nil? || uri.empty?
raise ArgumentError.new("Need a recipe uri pls")
end

Choose a reason for hiding this comment

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

Good work validating only things that will break your app if they're not there.

def self.raise_on_error(response)
unless response["count"]
raise RecipeError.new(response["error"])
end

Choose a reason for hiding this comment

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

This method is never called! Please don't copy-paste code without understanding how it fits together.

# ALWAYS CHECK YOUR ERROR CODES
# unless response.success?
# raise StandardError.new(response["error"])
# end

Choose a reason for hiding this comment

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

# ALWAYS CHECK YOUR ERROR CODES

ಠ_ಠ


it "displays the recipe page for the selected recipe" do
VCR.use_cassette("recipe") do
uri = "http://www.edamam.com/ontologies/edamam.owl#recipe_e427aa9f6e7a96ecadbf994d9f75fd14"

Choose a reason for hiding this comment

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

What about for a bad recipe uri?

describe 'index' do
it "returns a list of search recipes results" do
query = "chicken"
VCR.use_cassette("recipes") do

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:

  1. What happens if no search term is provided?
  2. What if there are no paging parameters provided?
  3. What if the paging parameters are invalid (negative number, etc)?
  4. What if the paging parameters take you past the end of the available search results?


it "can't give a list of recipes" do
VCR.use_cassette("recipes") do
recipe = RecipeApiWrapper.list_recipes("jdns2n")

Choose a reason for hiding this comment

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

You need some tests for show here! I would be interested in at least what happens when for a good vs bad uri.

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