Skip to content

Conversation

@auroralemieux
Copy link

@auroralemieux auroralemieux 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 read the docs and started trying things until something worked. I ended up using a recipes query with the q parameter and optional health parameters, and a recipe query with the r parameter. Postman was very useful throughout to see what the structure of the object being returned was (especially until I understood that the docs themselves were giving me the outline too).
Describe your API Wrapper. How did you decide on the methods you created? My EdamamApiWrapper was the go-between for my searches controller and the API. It has a searchQuery (great name, I know) class method that -- given a search term string, a from value, and a to value (and optional health param value) -- returns an array of RecipeResult objects as well as a hits count and a RecentSearch object that gets added to the recent searches array in session. It also has a class method that is much more straightforward and returns a single recipe given that recipe's uri. The searchQuery method got quite bloated in the process of adding extra functionality -- perhaps some of what's going on in there could be split out into other methods that get run before or called inside of it. Not sure.
Describe an edge case or failure case test you wrote for your API Wrapper. Yeah, about that. I have no idea how to write those tests. Testing was what I struggled with the most for this project and I'm going to get more help on it Tues and push some improved tests then. I think I may have bit off more than I could chew for the functionality and would have been better served spending my time getting the tests better.
Explain how VCR aids in testing an API. The VCR gem provides you with a way to run a search query multiple times to an API but really only have it ping the API once per unique query. This saves in cases where you're being charged per query, etc.
What is the Heroku URL of your deployed application? ala-recipe-rex.herokuapp.com (note that this is slightly different from my first PR url)
Do you have any recommendations on how we could improve this project for the next cohort? Ugh -- I needed more help with testing -- the rest of it seemed fairly straightforward comparatively.

PLEASE HELP ME WITH THESE QUESTIONS!

  1. My sessions controller create tests are not passing. I'm using procs in the wrong way, I think? Don't know!
  2. My searches controller seems really bloated! I'd love suggestions on how to dry it up and move things to other files.
  3. I had a hard time getting my tests to hit all of the session-related private methods in the searches controller. Next and Prev functionality was really hard to test. Don't think I got it quite right. Suggestions? I seemed to make a little progress after I realized I had to get the index page first to set all the session stuff that that action does.

THANK YOU!!

@CheezItMan
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene a TON of commits, and pretty good messages + 19 (19!) branches.
Comprehension questions Check, more stuff on testing to follow
General
Rails fundamentals (RESTful routing, use of named paths) Good routes, lots of extras here!
Semantic HTML Good semantic HTML
Errors are reported to the user Good use of the 404 page for bad URIs.
API Wrapper to handle the API requests Check
Controller testing By wrapping some of your tests in procs you're hiding some errors that are raised. I put a comment in one.
Styling
Foundation Styling for responsive layout Check, very nicely done, I like the look. Some of the images look funny at various sizes, but at standard sizes they look ok.
Search View Check
List View Check, nicely organized
Show View which opens in a new tab Check, well done
pagination Check
API Features
The App attributes Edaman Check
The VCR casettes do not contain the API key Check
1. Session Controller Tests Yes you are using procs in the wrong way, I put a comment in your code below showing you how.
2. Searches Controller Since you have a lot of options it tends to bloat the tests, since you have to test each one. You can use loops to verify that they work however. For example you could write a loop changing the from and to fields in the URL. You can also if your options like balanced etc are boolean (true/false) loop through it with an array of keys being used. In other words a loop through an array of [:vegetarian, :balanced, ...] running different requests. I'm not sure how the user can destroy a search in your site, so if it's not an option... Lastly your describe "previous" doesn't seem necessary.
3. next & previous testing You can test get and post etc requests with the parameters previous and next would set. That's it. Testing the specific buttons involves tools like Cappybara which are beyond the scope of Ada
Overall Very nice work on the filters! I would suggest using a password field for the user password so they type and get * to appear on the screen instead of the password's letters. There's a lot in here, very nicely done.

Copy link

@CheezItMan CheezItMan left a comment

Choose a reason for hiding this comment

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

I made one note on your controller testing, if you need me to look at more or other tests, just ping me back and I'll take a look.


describe "#create" do

it "can create a search" do

Choose a reason for hiding this comment

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

For this test case the proc is preventing errors from occurring, because `post login_path, params: {user: users(:aurora)} is raising an error because it's unable to find the user, but the proc is containing it. So the next 2 lines of the test never get executed.

You're also including search terms, health and user as params, but the controller is checking session for them (not sure why).

The test should look like

  it "can create a search" do
      post login_path, params: {
          provider: users(:aurora).provider,
          uid: users(:aurora).uid
      }
      must_respond_with :redirect
     proc {
        post save_search_path, params: {  
          search: { 
            search_terms: "pink lady", 
            health: "vegetarian",
            user_id: users(:aurora).id}
        }
    }.must_change 'Search.count', 1
      must_respond_with :success
    end

And I think from looking at it that the controller should be using params to save the search, not session. I'm not sure why session is involved at all.

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