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 - Rebecca - API Muncher #24

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

Conversation

rbergena
Copy link

@rbergena rbergena 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? I used postman, rails c, puts statements etc.
Describe your API Wrapper. How did you decide on the methods you created? I decided to create a recipes method that returns recipes meeting search criteria and a find recipe method that returns a specific recipe. These methods allow me to make requests to the API for all recipes meeting certain criteria and for specific recipes.
Describe an edge case or failure case test you wrote for your API Wrapper. I tested a case where bad tokens resulted in an api error.
Explain how VCR aids in testing an API. It allows you to store the results of an api call and use that information by storing it in a cassette. This allows you to create certain calls once without having to constantly make requests to the API when running tests.
What is the Heroku URL of your deployed application? https://recipe-muncher.herokuapp.com/

@PilgrimMemoirs
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Mostly Good - some commits should have a more specific message (ex: 'styling'). Also make sure to commit more often and break up work done into separate commits.
Comprehension questions Well Done
General
Rails fundamentals (RESTful routing, use of named paths) Well Done
Semantic HTML Well Done - Nice way of having the header partial show dynamically! You might appreciate this nifty rails helper, current_page?(https://stackoverflow.com/questions/31817051/ruby-on-rails-if-current-page-is-homepage-dont-show-form)
Errors are reported to the user Some Improvement - Flash errors don't appear to be showing, though they are being set in controller. No errors are shown if I don't put anything into search field and submit. (is the most recent version pushed to Heroku?)
API Wrapper to handle the API requests Mostly Good - make sure to have just the url for the base url constant, then append more specific parts of the url in the appropriate methods. Ex: BASE_URL = "https://api.edamam.com/". Also make sure you are using the api.edamam.com.
Controller testing Some Improvement - Good start, don't forget to test the status codes and redirects. All your controller tests are missing those.
Lib testing Well Done
Search Functionality Well Done
List Functionality Well Done
Show individual item functionality (link to original recipe opens in new tab)
Styling
Foundation Styling for responsive layout Well Done - responsiveness at all screen widths is solid on all 3 pages
List View shows 10 items at a time/pagination Well Done
The app is styled to create an attractive user interface Mostly Good - the form checkboxes are hard to read w/ the image background. I suggest having a background behind those checkboxes to make the text easier to read.
API Features
The App attributes Edaman Well Done
The VCR casettes do not contain the API key Well Done
External Resources
Link to deployed app on Heroku Well Done
Overall
Area for improvement: Status codes in the controller, and testing them. And displaying the flash messages to give user feedback.
Submission meets project expectations. Nice work!

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