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

Haby's API Muncher #58

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

Haby's API Muncher #58

wants to merge 42 commits into from

Conversation

habypsow
Copy link

@habypsow habypsow commented May 9, 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 first studied the documentation, and used postman to see how the data was organized, and to see what fields I needed to access for the project requirements. I then wrote the url method in the wrapper, and used better errors to access the data directly in the better errors environment.
Describe your API Wrapper. How did you decide on the methods you created? My API wrapper is using two methods, a search method with the 'recipe' keyword as a parameter, and then a show a single recipe method which uses the recipe's uri as a parameter to retrieve the recipe. I also had a recipe class which initialized both the required and optional parameters. I decided on using these methods because I was able to parse the response received from http party and set them to variables to store this data.
Describe an edge case or failure case test you wrote for your API Wrapper. I wrote a test that returns nil when an invalid recipe uri is passed as a parameter in the show recipe method.
Explain how VCR aids in testing an API. VCR's help to reduce the number of calls to the API, as the API data is stored as a fixture, and we are able to access this data in our tests, so that we do not need to make a new call for every test case.
What is the Heroku URL of your deployed application? https://haby-delish.herokuapp.com/
Do you have any recommendations on how we could improve this project for the next cohort? I found this project fun and enjoyable! So nothing I can think of :)

@CheezItMan
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good messages and lots of commits, nice use of branches in your coding.
Comprehension questions Check, glad you enjoyed the project!
General
Rails fundamentals (RESTful routing, use of named paths) You should consider paths that are simpler like /results and /recipe or /results and /results/recipe. There's no reason for 2 two-level paths. For example no one is going to go to /results or /recipe in your current routes file.
Semantic HTML You have a broken section closing tag in your layout file. Also nav is generally used for navigation menus. Since you just have the site title there, header is probably a better element to use. Otherwise pretty good semantic HTML. I would however take out some of the commented out <%# raise %> statements.
Errors are reported to the user Bad URIs for a dish give a generic rails error screen.
API Wrapper to handle the API requests Looks good, good use of the Recipe class.
Controller testing Slick use of before and after in your testing with VCR. However the search page doesn't need VCR since it's not using the API. You also need to test negative cases, any time variables might have incorrect values.
Lib folder testing Good work here, although you probably don't need to test specific values returned by the API (after all it might change. However you could test to make sure the fields are there. Good work on negative testing with the find recipe action.
Styling
Foundation Styling for responsive layout Looks really good! However it need to be responsive. You have in many area the number of columns each item has set to the same at small, medium and large sizes.
Search View Looks good
List View Looks good at large screen sizes
Show View which opens in a new tab Looks good at full screen size, but it doesn't scale well to smaller devices, using a grid layout like foundation would help. You simply need to make items take more columns at smaller screen sizes.
API Features
The App attributes Edaman Yes, but it doesn't link back to Edamam
The VCR casettes do not contain the API key Check
Overall Pretty good, you need more work on controller testing and using Foundation styling, but the site works and looks good.

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