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

Victoria Garcia - Ampers - API-Muncher #33

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

Conversation

Lasiorhine
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?

After spending lots of time poring over Edamam's own docs, I spent another large block of time poking at the API with Postman. Without Postman, I would have been totally lost.

Describe your API Wrapper. How did you decide on the methods you created? |

My API wrapper has two methods: one for getting search results and one for retrieving information about a specific recipe. I chose these because, well, that's what the assignment specified.

Describe an edge case or failure case test you wrote for your API Wrapper. |

For the recipe detail function, I wrote tests for if users try to key in an exogenous URI, instead of just clicking the button and sending a get request with the URI of one of the recipes displayed on the site.

Explain how VCR aids in testing an API. |

It creates a little pen in which you can test the behavior of your code against pre-recorded responses to previous API calls. That way, you don't have to make a fresh API call for every test you run.

What is the Heroku URL of your deployed application? |

https://victualizer.herokuapp.com/

Provide a link to the Trello board you used |

https://trello.com/b/jEnZutpQ/api-muncher

…back from the API. So the Recipe and EdamamApiWrapper are woring now, with not-so-great routes and views, and a recipes controller I'm not at all sure about. But this is better than a sock in the eye.
… make better routes and methods. Search and detail views added to recipes to facilitate this.
…ll a little goofy, but I don't think this is doomed anymore.
…lso, a few tweaks to the CSS, because having CSS means having to tweak it.
…if you want it to work on an external server.
… is a surprise FAIL after pull request submission, and I don't have time to ask Chris and Dee about it, and the test is more-or-less valid without it.
@CheezItMan
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits and good commit messages
Comprehension questions Check, but please don't break the markdown table when you're filling it out. If you do, just fix it.
General
Rails fundamentals (RESTful routing, use of named paths) The show path isn't restful
Semantic HTML Check
Errors are reported to the user Check, but hilarious messages
API Wrapper to handle the API requests Check
Controller testing Check
Lib testing Check
Search Functionality Check
List Functionality Check
Show individual item functionality (link to original recipe opens in new tab) Check, doesn't open in a new tab, but that's a minor quibble.
Styling
Responsive layout Somewhat responsive, it only adjusts from 3 to 4 columns, not any further. It does shrink content however. The show action works well however.
List View shows 10 items at a time/pagination Check
The app is styled to create an attractive user interface It look ok, good use of flexbox.
API Features
The App attributes Edaman Check
The VCR casettes do not contain the API key Check
External Resources
Link to Trello Board Check
Link to deployed app on Heroku Check
Overall Well done! You hit all the learning goals, nice work. One note being your show path isn't a RESTful route, but otherwise you got it all.

describe Recipe do

it "can be initialized" do
test_recipe = Recipe.new(

Choose a reason for hiding this comment

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

You should also have tests for the wrong # of arguments.

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