Pipes - Kimberley Zell - Api-Muncher by kimpossible1 · Pull Request #38 · Ada-C8/api-muncher

kimpossible1 · 2017-11-06T17:56:44Z

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 then the console to check the parsed response for my expectations
Describe your API Wrapper. How did you decide on the methods you created?	I followed along with the requirements. I created a self.search method that returns a list of recipes according to entry from user. I also made a self.find method that gets a specific recipe from the api, based on the uri of the recipe. I have a helper method in private methods that creates the individual recipe instances with only the data I have listed.
Describe an edge case or failure case test you wrote for your API Wrapper.	I checked that it returns nil if a recipe uri is invalid or not found.
Explain how VCR aids in testing an API.	It records data from a test query so that future tests on same query can use that saved information rather than query the API repeatedly
What is the Heroku URL of your deployed application?	https://recipe-monster.herokuapp.com/

…al_nutrients an array of only the key value pairs I want on my show page.

…empt to figure out how to add Foundation to my project.

…ndation file

…ed working

…changed the image to use the url

…recompile to see if that helps

droberts-sea · 2017-11-10T01:32:30Z

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	mostly - see inline comments
Lib testing	mostly - see inline comments
Search Functionality	yes
List Functionality	yes
Show individual item functionality (link to original recipe opens in new tab)	yes
Styling
Foundation Styling for responsive layout	yes
List View shows 10 items at a time/pagination	yes
The app is styled to create an attractive user interface	yes - In some places the text was hard to read on top of the background. One strategy to get around this is to make the element containing the text slightly opaque to give better contrast.
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	Good job overall! The core functionality of the app (querying an API and turning the results into a webpage) works great. Error handling isn't as fine-grained as I might like and (as with most software projects) there's some room to expand your test coverage, but in general I'm quite happy with this submission. Keep up the hard work!

droberts-sea · 2017-11-10T01:17:38Z

app/views/layouts/application.html.erb

+    </header>
+
+    <section class="main">
+      <% if flash[:message] %>


Why <section class="main"> instead of <main>?

droberts-sea · 2017-11-10T01:21:56Z

lib/recipe_api_wrapper.rb

+    response = self.parse_response(HTTParty.get(url))
+    recipe_list = []
+
+    if response.is_a?(Hash) && response["hits"]


I like these checks - they'll definitely keep your program form trying to read data that doesn't exist.

One thing I'm noticing: this method doesn't have any way to distinguish between an error response (for example because of bad API keys) and a success response with no hits.

Since one of these cases is a programer error and the other indicates the user did something wrong, I would argue that they ought to be handled differently. In order to do so, this method would have to return some more information.

droberts-sea · 2017-11-10T01:25:36Z

test/controllers/recipes_controller_test.rb

+    end
+
+    it "returns a collection of recipes when from parameter is not given" do
+      VCR.use_cassette("recipes") do


I like that you exercise both these cases around the from parameter. Other cases I'd be interested in seeing here:

What if the search returns no results (for example, a search for "dfaljdfalflaksjflaksj")?

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?

The first of those is covered by your lib tests below, but it's still important to verify the controller does the right thing in this situation.

droberts-sea · 2017-11-10T01:26:03Z

test/controllers/recipes_controller_test.rb

+
+    it "returns not found when an invalid uri is given" do
+      VCR.use_cassette("bad_uri_recipe") do
+        get recipes_path, params: { id: "http://www.edamam.com/ontologies/edamam.owl#recipe_421df807e21c65c842ec62870604" } #bad uri


Good failure test

droberts-sea · 2017-11-10T01:28:10Z

test/lib/recipe_api_wrapper_test.rb

+    end
+
+    VCR.use_cassette("recipe") do
+      recipe = RecipeApiWrapper.find(uri)


I like the pattern of this test - using real search results to seed the call to find.

droberts-sea · 2017-11-10T01:28:35Z

test/lib/recipe_api_wrapper_test.rb

+      result.must_be_kind_of Array
+      result.must_be_empty
+    end
+  end


I would like to see some testing around the second parameter to search here.

kimpossible1 added 19 commits October 30, 2017 21:50

some basic set up

9431610

trying to get set up

8d7152a

lots of updates

0ace9f4

completed basic show page for recipe. Added to ApiWrapper to make tot…

7e14ce0

…al_nutrients an array of only the key value pairs I want on my show page.

Just before this commit, I added the gem for foundation. Now will att…

3852143

…empt to figure out how to add Foundation to my project.

footer etc

11ade3e

trying to get vcr working

469657e

got tests working, there was an issue with gemfile. deleted extra fou…

e75a24d

…ndation file

mainly styling. For some reason the background behind my images stopp…

5f09aac

…ed working

styling

3ef53ec

Finished wrapper tests

d94ab60

finished controller testing

e6becff

updated styling

c23811c

added gem for heroku

5f20899

added require to top of controller file'

b996401

adds config assets to production

06d1c25

adds image url

09ce67d

unable to get heroku to use my background image from assets, so have …

e78064f

…changed the image to use the url

changed image to url but heroku still did not show it. tryig to run p…

242cc45

…recompile to see if that helps

droberts-sea reviewed Nov 10, 2017

View reviewed changes

kimpossible1 added 7 commits December 6, 2017 18:40

fixed a bug

dba954b

Update README.md

370cf82

updating readme

a61db0e

readme

a55c216

updated Readme

2b76453

readme update

5cc3f54

updated readme

a8f1056

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pipes - Kimberley Zell - Api-Muncher#38

Pipes - Kimberley Zell - Api-Muncher#38
kimpossible1 wants to merge 26 commits intoAda-C8:masterfrom
kimpossible1:master

kimpossible1 commented Nov 6, 2017

Uh oh!

droberts-sea commented Nov 10, 2017

Uh oh!

droberts-sea Nov 10, 2017

Uh oh!

droberts-sea Nov 10, 2017

Uh oh!

droberts-sea Nov 10, 2017

Uh oh!

droberts-sea Nov 10, 2017

Uh oh!

droberts-sea Nov 10, 2017

Uh oh!

droberts-sea Nov 10, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

kimpossible1 commented Nov 6, 2017

API Muncher

Comprehension Questions

Uh oh!

droberts-sea commented Nov 10, 2017

API Muncher

What We're Looking For

Uh oh!

droberts-sea Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

droberts-sea Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

droberts-sea Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

droberts-sea Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

droberts-sea Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

droberts-sea Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants