Jamila Octos c9 #36

Jcornick21 · 2018-05-07T15:23:23Z

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 read through the documentation tried out different queries in postman and looked at the data structure of the response
Describe your API Wrapper. How did you decide on the methods you created?	I started with what I needed which was a method to display the initial query and then after quite some time realized that I would need a separate method to do a showpage.
Describe an edge case or failure case test you wrote for your API Wrapper.	Not really an edge case just wrote to raise an error if the query is empty.
Explain how VCR aids in testing an API.	It makes it so I am not calling to the api every time I run tests that involve the api directly and indirectly(like through routes in the controller)
What is the Heroku URL of your deployed application?	didnt deploy
Provide a link to the Trello board you used	https://trello.com/b/7GBVLw0L/api-muncher

Jcornick21 · 2018-05-07T15:27:23Z

I know I didnt get to controller tests and some of my other tests have probably broken due to adding the show page functionality. I also needed and wanted to add a test to the wrapper tests for the new method I created. I meant to at least put some flex css for the pagination and make my form view into a partial so I could better place it when I added more css later.

droberts-sea · 2018-05-10T21:32:51Z

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	no - when I mangle the recipe URI, I get an exception rather than a polite error message
API Wrapper to handle the API requests	yes
Controller testing	no
Lib testing	yes - a couple are missing but in general this looks very good
Search Functionality	yes
List Functionality	yes
Show individual item functionality (link to original recipe opens in new tab)	yes
Styling
Responsive layout	yes
List View shows 10 items at a time/pagination	yes
The app is styled to create an attractive user interface	no
API Features
The App attributes Edaman	no - This is really important! When you're designing code for money, using someone else's work without attribution is a good way to end up in legal trouble.
The VCR casettes do not contain the API key	yes
External Resources
Link to Trello Board	yes
Link to deployed app on Heroku	no
Overall

Good work overall. It's clear that the the core learning goal for the assignment around interacting with an API was met. I also really like your lib tests - good job on this!

There are two places I see room for improvement. The first is in error handling - making sure that you've got a comprehensive story for how errors get from the bottom layer (the API) all the way back to the user is not easy, but it's a piece of what sets apart a really strong application. The other place is controller testing, which you're aware of. Fortunately these are both major components of the VideoStore API project - please spend some time focusing on these concepts this week.

I've seen you putting in a large amount of work recently, and I think it really shows on this assignment. This submission represents a big improvement over where you were a couple weeks ago. Keep it up!

droberts-sea · 2018-05-10T21:12:02Z

config/routes.rb

+  root 'edamams#new'
+  get '/recipe/new', to: 'edamams#new'
+  get '/recipe/index', to: 'edamams#index', as: 'recipe_list'
+  get '/recipe/:uri', to: 'edamams#show', as: 'recipe_details'


I'm not sure that new is a good name for this route/action. You're not really creating a new recipe, you're doing a search.

Also, could you use resources for the index and show pages?

droberts-sea · 2018-05-10T21:14:00Z

lib/recipe.rb

+    raise_error(source)
+    raise_error(uri)
+    raise_error(url)
+    raise_error(ingredientLines)


I would say you're over-validating here. In general you should only validate fields that will cause your app to break if they're not there. In this case that's label, uri and possibly image. Anything beyond that is an unnecessary dependency on the data coming from the API, and is an extra thing for you to test.

droberts-sea · 2018-05-10T21:15:11Z

lib/recipe_api_wrapper.rb

+  def self.raise_on_error(response)
+    # this needs to be changed based on the api
+    unless response["more"]
+      raise RecipeError.new(response["error"])


I'm not sure that Edamam gives error messages back under the error key like this.

droberts-sea · 2018-05-10T21:16:27Z

lib/recipe_api_wrapper.rb

+    full_uri = URI.encode("https://api.edamam.com/search?r=#{BASE_URI}#{uri}&app_id=#{TOKEN_ID}&app_key=#{TOKEN_KEY}")
+
+     response = HTTParty.get(full_uri).parsed_response
+     return Recipe.from_api(response[0])


You should check that the API call succeeded before calling Recipe.from_api here.

droberts-sea · 2018-05-10T21:17:43Z

app/controllers/edamams_controller.rb

+  def index
+    recipe_list = RecipeApiWrapper.recipes(params[:recipe])
+    @recipe_list = Kaminari.paginate_array(recipe_list).page(params[:page]).per(10)
+  end


You've written your API wrapper to raise an error if the API call fails, but you're not looking for an error here. You should wrap this call in a begin/rescue.

droberts-sea · 2018-05-10T21:19:52Z

test/lib/recipe_api_wrapper_test.rb

+
+  it "raises and error if query is empty" do
+    VCR.use_cassette('recipes') do
+      query = ' '


Good error case

droberts-sea · 2018-05-10T21:21:34Z

test/lib/recipe_api_wrapper_test.rb

+  describe "single recipe" do
+    it "" do
+
+    end


There are two tests I would want to see here:

Do you get an instance of class Recipe back with a valid URI?

What happens when you pass in a bogus URI?

droberts-sea · 2018-05-10T21:22:35Z

test/controllers/edamams_controller_test.rb

+
+describe EdamamsController do
+  # it "must be a real test" do
+  #   flunk "Need real tests"


You need some controller tests! Here is my list of what I'm looking for:

Index:

Valid search term

Search term that produces no results (e.g. adfohdaglhjalj)

No search term

Show:

Valid URI

Bogus URI

No URI

Root

Does it work at all (only one test)

Jcornick21 added 13 commits May 2, 2018 15:17

rails new, added dotenv, added httparty

4f42779

added keys to .env file

9a5f0b7

modified recipe wrapper

d30ff3e

generated controller

4c3c895

built out view modiefied controller and routes

f74548c

added test file for lib

e6cac72

wrote tests for recipe.rb

831b007

wrote tests for recipe.rb

590a39b

tests passing for recipe_tests, refactored recipe.rb

2746e14

tests for first method in api_wrapper modified api wrapper

c8813b4

recipe search works and images show in view

68c2760

added pagination

565ba93

got show functionality working

1899f2d

Jcornick21 added 2 commits May 7, 2018 08:59

details in show and recipe class to make the show view actually work

8bfed4d

modified recipe.rb

468fed4

droberts-sea reviewed May 10, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jamila Octos c9 #36

Jamila Octos c9 #36

Jcornick21 commented May 7, 2018

Jcornick21 commented May 7, 2018

droberts-sea commented May 10, 2018

droberts-sea May 10, 2018

droberts-sea May 10, 2018

droberts-sea May 10, 2018

droberts-sea May 10, 2018

droberts-sea May 10, 2018

droberts-sea May 10, 2018

droberts-sea May 10, 2018

droberts-sea May 10, 2018

Jamila Octos c9 #36

Are you sure you want to change the base?

Jamila Octos c9 #36

Conversation

Jcornick21 commented May 7, 2018

API Muncher

Comprehension Questions

Jcornick21 commented May 7, 2018

droberts-sea commented May 10, 2018

API Muncher

What We're Looking For

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment