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

Chantelle Edges Api Muncher #33

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

Conversation

BASIC-Belic
Copy link

@BASIC-Belic BASIC-Belic commented Nov 5, 2018

API Muncher

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How did you go about exploring the Edamam API, and how did you try querying the API? I read the documentation, but to be honest, at times this was misleading. I also tried it out in rails c, and Postman to see how requests and results are formatted.
What is an API Wrapper? Why is it in lib? How would your project change if you needed to interact with more than one API (aka more than just the Edamam API)? It's in lib because it doesn't fit the role of controller or model. An API Wrapper receives and sends the data from an API in a usable way to your controller. If we had to use many API's, then we'd need wrappers for each of those API's since the data, status code handling, etc would be different.
Describe your API wrapper, the methods you created in it, and why you decided to create those methods/how they were helpful In my wrapper I included a method to list recipes that started by constructing the url to request a list of recipes, then used HTTParty to request the data which was stored in an array. I created another method to show the details of a specific recipe that also constructed a URL and used HTTParty to request data. I also created two private helper methods, one to create a recipe for use in the list recipes method, and one to create the details of a recipe for use in the show details method.
What was an edge case or failure case test you wrote for your API Wrapper? What was a nominal case? I only wrote the nominal for if there's a successful search so far.
How does VCR aid in testing an API? The VCR records your first response to a yaml file so that you can use that data without having to make a bunch of expensive calls to the API.
What is the Heroku URL of your deployed application? http://chantelle-api-muncher.herokuapp.com/

@CheezItMan
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits and good commit messages
Comprehension questions Check, well done
General
Rails fundamentals (RESTful routing, use of named paths) Check
Semantic HTML Lots of good uses of partials. Just note you can use other container tags like section, article, main etc when you use Bootstrap classes. They don't have to be div elements.
Errors are reported to the user Check, well done, but if I submit the wrong uri to the show action the app crashes.
API Wrapper to handle the API requests Check, but I would make private any methods you don't want to expose.
Controller testing MISSING
Lib testing Good testing
Search Functionality Check
List Functionality Check
Show individual item functionality Check
Styling
List view shows 10 items at a time and/or has pagination It seems to show all the items with no pagination
Attractive and usable UI Simple UI, but it works, good responsiveness on the search results page.
API Features
The App attributes Edamam Check
The VCR cassettes do not contain the API key Check
External Resources
Link to deployed app on Heroku Check
Overall You hit the major learning goals, but you missed having controller testing and you should also have feedback to the user for a failed show action.

<div class="col-sm">
<ul>
<li><strong><%= recipe.name %></strong></li>
<li><%= image_tag recipe.image_url %></li>

Choose a reason for hiding this comment

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

Just a note, it's good to make the images links because it's human nature to want to click on the images.

else
#render not found view
#flash error
head :not_found

Choose a reason for hiding this comment

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

So you're giving only the head in the response and not rendering any page. You should probably render something to the user.

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