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.

Sign up for GitHub

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

Jump to bottom

Pipes - Sairagul - Muncher #48

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

Pipes - Sairagul - Muncher #48

wants to merge 13 commits into from

Conversation

sairagula
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? I used Postman and rails console with curl
Describe your API Wrapper. How did you decide on the methods you created? I created 2 methods, search (that returns all recipe data from API) and find (returns data of a particular recipe). Also private method that creates Recipe instances for both search and find methods
Describe an edge case or failure case test you wrote for your API Wrapper. Tested for invalid uri id, it will raise an error
Explain how VCR aids in testing an API. It stores the results of a request in a cassette and uses that information for future tests without having to constantly make requests to the API
What is the Heroku URL of your deployed application? https://muncher-sairagul.herokuapp.com/

@PilgrimMemoirs
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Some Improvement - Should be committing more often
Comprehension questions Well Done
General
Rails fundamentals (RESTful routing, use of named paths) Needs Improvement - Routes should follow RESTful conventions - Index should always be the list of a resource (instead of results, in this case). The show route should start with the resource name, then the id, like: 'recipes/:id'
Semantic HTML Some Improvement - <body> tag should only be used once, and is already in the application.html.erb (should not be in any other page). Instead, <main> is often more appropriate to wrap around page specific content, if it's the main content of the page. Show page should utilize more semantic sectioning tags, instead of divs.
Errors are reported to the user Not Complete - Remember to utilize flash messages to inform user of unexpected results, like no recipes being found from a search query. None of the flash messages set are showing in browser.
API Wrapper to handle the API requests Well Done
Controller testing Well Done
Lib testing Well Done
Search Functionality Well Done
List Functionality Mostly Good - Should handle if no recipes are found
Show individual item functionality (link to original recipe opens in new tab) Well Done
Styling
Foundation Styling for responsive layout Some Improvement - show pages not responsive, should utilize foundation grid to do so.
List View shows 10 items at a time/pagination Well Done
The app is styled to create an attractive user interface Mostly Good - list of recipes has last recipe floating to right, I recommend using foundation's block grid to avoid that.
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
Areas for Improvement Following REST conventions when building routes and controller methods, Using semantic HTML to organize content, Displaying messages to user using flash, and using foundation to make all pages responsive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sairagula @PilgrimMemoirs