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

Pipes - Bennett - Muncher #41

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

Conversation

bennettrahn
Copy link

@bennettrahn bennettrahn commented Nov 6, 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? Using postman and trying out some of the different request variables on their documentation.
Describe your API Wrapper. How did you decide on the methods you created? I made one for index/search using the -q, and one for finding a specific show/id using -r
Describe an edge case or failure case test you wrote for your API Wrapper. if the url was invalid, or the search terms were invalid
Explain how VCR aids in testing an API. it saves queries so you don't have to call the api every time you run rails test, you just call the stashed data call.
What is the Heroku URL of your deployed application? https://muncher-bennett.herokuapp.com/

This does not represent a necessarily finished project, I didn't finish the pagination or various other things, but its good enough and I'm done. For now.

@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 (doesn't open in new tab)
Styling
Foundation Styling for responsive layout yes
List View shows 10 items at a time/pagination some - no pagination
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 deployed app on Heroku yes
Overall Looks good overall.

url = BASE_URL + "q=" + query + ID_AND_KEY + paginate(from)
data = HTTParty.get(url)
unless data["hits"].empty?
data["hits"].each do |hit_data|

Choose a reason for hiding this comment

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

You shouldn't need the unless statement here - if data["hits"] is empty, then data["hits"].each will iterate zero times.

must_redirect_to root_path
flash[:message].must_equal "Invalid input: please try again without symobls."
end
end

Choose a reason for hiding this comment

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

I know pagination isn't working, but there are a few test cases around it that I would like to see here:

  • Make sure paging parameters are accepted, and that requesting a different page doesn't break things
  • What if the paging parameters are invalid (negative number, etc)?
  • What if the paging parameters take you past the end of the available search results?

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