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

Ari- Octos- Muncher #44

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

Ari- Octos- Muncher #44

wants to merge 21 commits into from

Conversation

arrriiii
Copy link

@arrriiii arrriiii commented May 7, 2018

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 was able to read through the documentation and utilize postman for what attributes to display. How to handle the uri for the recipe_detail method was difficult. -- Although I did not get to css, I enjoyed working through this project.
Describe your API Wrapper. How did you decide on the methods you created? The documentation for Edamam required a q= for query searches and r= for a recipe id, so I created two methods for each url; The recipe_detail method to return the details of a single recipe and a recipe_list for a list of recipes with the given query.
Describe an edge case or failure case test you wrote for your API Wrapper. I wrote a case for when the recipe_list method that would raise an error when the query DNE-- the test failed not because I didn't raise a standard error but b/c of my argument error in my recipe.rb file. I found that the failing test was because one of the parameters in from_api method was spelled incorrectly. > ingredientlines was actually ingredientLines<
Explain how VCR aids in testing an API. VCR records the HTTP calls and uses that information to mimic an Api in the test environment. -- This way, testing is not relied on the db and limits the calls to the api.
What is the Heroku URL of your deployed application? Unfortunately, wasn't able to deploy to Heroku
Provide a link to the Trello board you used https://trello.com/b/3GNIhB87/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 - see inline
API Wrapper to handle the API requests yes - good work
Controller testing no
Lib testing yes - good work identifying failure cases
Search Functionality yes
List Functionality yes
Show individual item functionality (link to original recipe opens in new tab) yes (link does not pop out)
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
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 no
Overall This is a really strong start. Your lib files and corresponding tests are very solid, and it's clear that the learning goals of this assignment around working with an API were met. However while the app mostly works, there are problems around error handling and your controller tests are missing entirely, which are important parts of our overall Rails curriculum. Please focus on these as you work on VideoStore API, and keep up the hard work.

root 'grub#new'
get '/recipe', to: 'grub#index', as: 'grub_list'
get '/recipes/:uri', to: 'grub#show', as: 'grub'
get '/recipe/new', to: 'grub#new'

Choose a reason for hiding this comment

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

Two things here:

  • I'm not sure that new is a good description of what this route is doing - something non-RESTful like search might be a better name
  • Could you use resources for the index and show routes?

rescue EdamamApiWrapper::RecipeError => error
flash[:status] = :failure
flash[:message] = "API called failed: #{error}"
redirect_back fallback_location: root_path

Choose a reason for hiding this comment

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

Here you add an error to the flash, but the code in your view to display flash errors doesn't match this format. This means that the user gets redirected to the root page with no explanation of what went wrong - not a great experience.

APP_ID = ENV["APP_ID"]
BASE_URL = "https://api.edamam.com/search?"
URI_BASE = "http://www.edamam.com/ontologies/edamam.owl#recipe_"
FROM = 1

Choose a reason for hiding this comment

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

BASE_URL and URI_BASE are very visually similar, and it's not immediately clear how they're different. Could you rename one or both to something more distinct?

response = HTTParty.get(encoded_uri)

raise StandardError.new("Could not find recipe") if response == []

Choose a reason for hiding this comment

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

You do a good job of checking for errors in this file!

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