-
Notifications
You must be signed in to change notification settings - Fork 47
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
LJ API Muncher #24
base: master
Are you sure you want to change the base?
LJ API Muncher #24
Conversation
API MuncherWhat We're Looking For
|
Rails.application.routes.draw do | ||
root "recipes#index" | ||
get 'recipes/index', to: "recipes#index", as: "list_recipes" | ||
get 'recipes/show/:find', to: "recipes#show", as: "show_recipe" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use resources
for the index and show pages?
api_params["ingredientLines"], | ||
api_params["healthLabels"], | ||
api_params["cautions"] | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to merge show_recipe
and create_recipe
?
def show | ||
if params[:find] | ||
@find = params[:find] | ||
@recipe = EdamamApiWrapper.find_recipe(@find) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
.
Not doing so affects your ability to catch fail states and properly redirect the user.
http_interactions: | ||
- request: | ||
method: get | ||
uri: https://api.edamam.com/search?app_id=<MUNCHER_TOKEN>&app_key=04c236eb15ab0bae3dd07c08766aa823&r=http://www.edamam.com/ontologies/edamam.owl%23recipe_7bf4a371c6884d809682a72808da7dc2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nooooooo app keeeeeyyyyyyy
it "should get home" do | ||
# make test to | ||
get root_path | ||
value(response).must_be :successful? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some interesting cases you're not covering here:
- What happens if no search term is provided?
- What if the search returns no results?
It might also be worthwhile to add some tests around the paging parameters:
- What happens if they're absent?
- Do you get different results when they change?
- What if you send a bogus value, like a negative page number?
|
||
it 'can find recipe given a valid path' do | ||
VCR.use_cassette('find_valid') do | ||
find = "http://www.edamam.com/ontologies/edamam.owl#recipe_7bf4a371c6884d809682a72808da7dc2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if they give you a bad recipe?
API Muncher
Congratulations! You're submitting your assignment!
Comprehension Questions
lib
? How would your project change if you needed to interact with more than one API (aka more than just the Edamam API)?