-
Notifications
You must be signed in to change notification settings - Fork 42
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
Pipes - angela - api muncher #26
base: master
Are you sure you want to change the base?
Conversation
… Recipe class. still working on associated tests.
…n controller from command line.
…links to recipe details, view incomplete.
…mbol is not permitted
API MuncherWhat We're Looking For
|
def index | ||
@query = params[:search] | ||
|
||
if params[:search] |
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.
Because the empty string is truthy in Ruby, even if you search for nothing you won't end up in the error handling here. Instead, you might have something like if params[:search] && params[:search] != ""
.
|
||
def new | ||
|
||
end |
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.
I don't think the new
action is used in your app.
|
||
unless @recipe | ||
head :not_found | ||
end |
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.
I like this idea, but I do not believe the code will ever get here. Your API wrapper throws an error if the recipe is not found, but here you check for a nil
response. You have two options to make the connection:
- Use a
begin
/rescue
block here to catch the exception - Instead of throwing an error, return
nil
fromApiWrapper.find_recipe
if the recipe can't be found.
|
||
|
||
# parse the JSON data in order to get recipe details for the show page | ||
if data.empty? |
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.
It looks like you're missing a call to check_status
here. If the API returns an error like not_authed
(as opposed to no results), as is this method won't be able to detect it.
# # | ||
# | ||
# get recipe_path params: { search: "chicken", url: "http://www.edamam.com/ontologies/edamam.owl%23recipe_637913ec61d9da69eb451818c3293df2" + "fake-url"} | ||
# must_respond_with :not_found |
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.
I think this test is failing because the controller action is handling the error incorrectly. See comments above.
begin # something which might raise an exception | ||
origin_token = ApiWrapper::TOKEN | ||
ApiWrapper::TOKEN = "bogus_token" | ||
VCR.use_cassette("recipes") do |
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.
I like this way of testing an invalid token - much simpler than what we did in class.
API Muncher
Congratulations! You're submitting your assignment!
Comprehension Questions