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

Brenda Rios - API-Muncher - Octos #25

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

Conversation

brendarios
Copy link

@brendarios brendarios 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 checked the Edamam documentation and used Postman to test request paths.
Describe your API Wrapper. How did you decide on the methods you created? My API Wrapper consists of two methods, list_of_recipes and show_recipe. With the method list_of_recipes using a search term I am able to retrieve a list of recipes, then using its uri I am able to retrieve the details for a specific recipe.
Describe an edge case or failure case test you wrote for your API Wrapper. An edge case I testes was that it returns nil with a bogus recipe uri. Also tested that it returns an error if bogus arguments in the show recipe method.
Explain how VCR aids in testing an API. VCR aids in testing because by saving API responses in the cassettes, it reduces the number of API calls we make.
What is the Heroku URL of your deployed application? https://munchmore.herokuapp.com/
Provide a link to the Trello board you used https://trello.com/b/a6jav2aU/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 yes
API Wrapper to handle the API requests yes
Controller testing yes
Lib testing yes
Search Functionality yes
List Functionality yes
Show individual item functionality (link to original recipe opens in new tab) yes
Styling
Responsive layout yes
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 yes
Overall Great job overall! Your code is well-organized and easy to read, and it's clear the learning goals for this assignment were met. I've left a couple of comments inline, but in general I'm quite happy with this submission. Keep up the hard work!

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

# path for showing details of a recipe
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 these two routes?

</body>

<footer>
<h6>Powered by</h6>

Choose a reason for hiding this comment

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

The <footer> (and all content that renders on the page) should go inside the <body>


describe "#index" do

it "successfully loads recipe results for any search term" do

Choose a reason for hiding this comment

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

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?

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