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

Ashton Fridge Muncher #62

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

Ashton Fridge Muncher #62

wants to merge 22 commits into from

Conversation

ashtn
Copy link

@ashtn ashtn commented May 10, 2017

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 explored the Edamam API through post man tools, paying close attention to the url syntax, what data was being returned, and how that data was organized. I also explored the API via edamam.com
Describe your API Wrapper. How did you decide on the methods you created? My Api wrapper consists of two methods, I was hoping to only use one because the initial search query, which returns a list of recipes based on user input, returns enough information for the show page, but I found it difficult to pass data through to the next request cycle without a model. Thus, I have one method to find a list of recipes based on a search query, and another to return details of a specific recipe based on the recipes uri. Both methods create a url, initializes a call to the api, parses through response data, and returns new recipe objects.
Describe an edge case or failure case test you wrote for your API Wrapper. An edge case I wrote, tested the showRecipe method with a bad uri, simulating a broken link on the results page, or bad user input data.
Explain how VCR aids in testing an API. VCR works like the or operator in ruby. If a certain api called has been recorded, instead of making a new API it plays back the content from the recorded cassette, else it will make a new API call and then record that response (even a bad response)
What is the Heroku URL of your deployed application? http://fridgemuncher.herokuapp.com/
Do you have any recommendations on how we could improve this project for the next cohort? Not at this time

@CheezItMan
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Enough commits, good messages, nice use of Branches!
Comprehension questions Check, It's hard, to send data from one request to another without a model. You made the right choice there.
General
Rails fundamentals (RESTful routing, use of named paths) sensible routes
Semantic HTML You have nav block that functions more like a header. For the layout div is fine, although you don't HAVE to use div. You can use other tags like section, article, body etc.
Errors are reported to the user Nope, on bad URIs, it just gives the generic rails error screen.
API Wrapper to handle the API requests Check, not sure why you have, puts "Whoops, That's embarrasing, but we couldn't find that recipe. #{e}" in the API wrapper however.
Controller testing Check, no test for a bad URI on the recipe show page.
Styling
Foundation Styling for responsive layout Nice responsive layout, it looks really nice, you have an eye for this. I would make the food item text bigger and more prominent, it' a big hard to read. I like how you indicated the nutritional information.
Search View Check, interesting background
List View Check, looks nice and the images are clickable.
Show View which opens in a new tab Check, although the link to the origin doesn't open in a new tab.
API Features
The App attributes Edaman Check
The VCR casettes do not contain the API key You do have the API key in the casettes! Check your test_helper.rb!
Overall This was really nice, you did an excellent job on this. A few minor things here and there, but it looks good and it's well written. Just remember to test negative cases!

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